Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Accessible Autocomplete a GOV.UK Prototype Kit plugin #598

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

joelanman
Copy link
Contributor

  • adds a config file to make the JS and CSS available to the kit as a plugin
  • adds info to the Readme so people know how to use it as a plugin

@joelanman joelanman marked this pull request as ready for review August 31, 2023 10:39
@joelanman
Copy link
Contributor Author

are there any changes I need to make to this?

@joelanman
Copy link
Contributor Author

might need guidance or preferably code to style it like gov.uk

@joelanman
Copy link
Contributor Author

Hi is this something I can help with or shall I close it?

@colinrotherham
Copy link
Contributor

@joelanman I've rebased this PR for you and we've added it to the "After next" milestone

You'll see one extra commit 0be7e76 to avoid alphagov/govuk-prototype-kit#1950

Looks good to me

@joelanman
Copy link
Contributor Author

joelanman commented Jan 18, 2024

@colinrotherham cheers! The only other thing I can think of is that the autocomplete isn't styled to GOV.UK by default (the font is the most obvious difference). I wonder if it's easy to make that happen when it's installed as a plugin, or if we just give people instructions on how to do it.

Related:

@colinrotherham
Copy link
Contributor

@joelanman Ah good thinking, forgot I replied to that one back in the day

We can review whether GDS Transport is allowed within .govuk-form-group only perhaps

But I've pushed up some of those tweaks to fix #285 too

@joelanman
Copy link
Contributor Author

@colinrotherham nice! Just to check is the intention to

  • apply some non branded but 'nice' default styling
  • add gov.uk styling if the autocomplete is inside a govuk-form-group class?

If so, that seems nice, but I think there's a danger people don't use the govuk-form-group class.

My idea is to add another css/sass file purely for gov.uk branding. In the kit this could be included by default (via the plugin config) - others could opt in manually (with instructions in the readme). What do you think?

@colinrotherham
Copy link
Contributor

@joelanman Yeah exactly that

I've made sure the defaults use Arial and align with GOV.UK Frontend

Prototype Kit plugin stylesheet sounds like a great idea if there's no nice way to opt-in GOV.UK services too

@romaricpascal
Copy link
Member

romaricpascal commented Jan 19, 2024

Anticipating some breaking changes from Colin making the autocomplete catch-up with GOV.UK Frontend's style, we're going to split them in their own PR we can add to the current release, which already brings some breaking changes.

I think I'd be keen to have the styling currently set through .govuk-form-group (and only that part necessary to integrate with the Prototype Kit) in its own stylesheet specific for the Prototype Kit, and listed in the plugin.

That being said, I'm also wondering if that's not something we could solve by making sure you can control the fonts inside the component by setting a class on its root element (or a wrapper) and let the values be inherited when necessary. Will give it a quick try.

@colinrotherham
Copy link
Contributor

I've moved the CSS tweaks to #644

We're still unsure on how best to serve a separate stylesheet (or styles) for GOV.UK services

</select>
</div>
```

Copy link
Contributor Author

@joelanman joelanman Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think relying on the class is a bit fragile, but if thats the approach I would suggest calling it out, something like:

Note that the class govuk-form-group is required to make the autocomplete look like GOV.UK.

Because if they use examples from elsewhere, it won't have it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joelanman
Copy link
Contributor Author

is there anything I can help with on this?

@romaricpascal
Copy link
Member

is there anything I can help with on this?

@joelanman, thanks for your patience, we'd scheduled this PR for further review after the next release of the accessible autocomplete, which just happened.

I think the main thing that remains to sort is how to provide a default styling for the accessible autocomplete when used in the Prototype Kit. I'd personally be keen to avoid coupling the accessible autocomplete to GOV.UK Frontend's classes and avoid having styles specific to the Prototype Kit in the 'default' CSS which will be included outside of the Prototype Kit. Acknowledging that this would mean building a new, separate, stylesheet so will double check with the team.

Comment on lines +155 to +162
/* Try font "GDS Transport" for GOV.UK form groups */
.govuk-form-group .autocomplete__hint,
.govuk-form-group .autocomplete__input,
.govuk-form-group .autocomplete__option {
font-family: "GDS Transport", arial, sans-serif;
-webkit-font-smoothing: antialiased;
-moz-osx-font-smoothing: grayscale;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add to what @romaricpascal was saying:

This could instead be part of a separate autocomplete-govuk.css (or better named) stylesheet that is referenced in govuk-prototype-kit.config.json.

That way autocomplete users can choose from:

  • GOVUK styling autocomplete-govuk.css
  • Default styling autocomplete.css
  • No styling (DIY)

Also wanted to mention: the historic reason for why the autocomplete doesn't use GOVUK Frontend classes is because it precedes v0.1 of GOVUK Frontend. It's also a nice feature which has allowed it to be adopted in places outside Gov such as FT or Wordpress. So yes, keeping the "Default styling" agnostic is still worth doing. 👍

Copy link
Contributor Author

@joelanman joelanman Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this might work:

.govuk-template .autocomplete__hint,
...
  • simpler to have one css file, people working on govuk things don't have to think about config
  • one request is better for performance
  • .govuk-template is always present in the Prototype Kit (and many other GOV.UK projects), whereas there may not be a parent .govuk-form-group

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs review 🔍
Development

Successfully merging this pull request may close these issues.

4 participants