-
Notifications
You must be signed in to change notification settings - Fork 149
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
base: main
Are you sure you want to change the base?
Conversation
joelanman
commented
Aug 30, 2023
- 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
f45efef
to
5188931
Compare
20c67b1
to
11e1baa
Compare
are there any changes I need to make to this? |
might need guidance or preferably code to style it like gov.uk |
Hi is this something I can help with or shall I close it? |
11e1baa
to
0be7e76
Compare
@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 |
@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: |
@joelanman Ah good thinking, forgot I replied to that one back in the day We can review whether But I've pushed up some of those tweaks to fix #285 too |
@colinrotherham nice! Just to check is the intention to
If so, that seems nice, but I think there's a danger people don't use the 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? |
@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 |
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 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. |
77c5ba8
to
247e72a
Compare
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 |
Only add “GDS Transport” to the font stack when using `.govuk-form-group` wrappers
247e72a
to
0923a5d
Compare
</select> | ||
</div> | ||
``` | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it could get a mention here too?
https://github.com/alphagov/accessible-autocomplete?tab=readme-ov-file#styling-the-autocomplete
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. |
/* 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; | ||
} |
There was a problem hiding this comment.
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. 👍
There was a problem hiding this comment.
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