alphagov / govuk-design-system

One place for service teams to find styles, components and patterns for designing government services.
https://www.gov.uk/design-system
MIT License
512 stars 232 forks source link

Add styling for site search input 'flowing' into dropdown #4220

Closed owenatgov closed 3 weeks ago

owenatgov commented 1 month ago

Change

Adds a visual cue to the site search so that users whre zoomed into the input know that there is a dialog below that they may be able to interact with. When a dialog is displayed, the input will be given 'bottomless' styling to remove it's bottom border, making it visually flow into the menu and drawing the eye down.

This applies any time there's a visible drop down, so for any number of results and when the 'no results' dialog displays.

Fixes https://github.com/alphagov/govuk-design-system/issues/4015

Visual changes

Scenario Before After
Input focused Focused site search without bottomless styling Focused site search with bottomless styling
Input not focused (theoretically impossible without messing with inspect) Unfocused site search without bottomless styling Unfocused site search with bottomless styling
Forced colours (used Firefox's dark theme) Site search in forced colours mode without bottomless styling Site search in forced colours mode with bottomless styling

Notes

This PR follows https://github.com/alphagov/accessible-autocomplete/pull/753. See https://github.com/alphagov/accessible-autocomplete/pull/753#issuecomment-2407259328 for testing notes from that PR.

There's a little gap between the fake :before border at the top of the dropdown and the first item's top edge when highlighted. This is becuase we found through testing that some forced colour modes set the colour of the :before element to be the same ass the background colour of the dropdown, effectively erasing it. If the top item is at the very top of the dropdown, right under the input, and is highlighted, it looks like the bottom border of the input making this effect even more subtle. The gap helps with drawing the eye down.

netlify[bot] commented 1 month ago

You can preview this change here:

Name Link
Latest commit ee7ec341ceec9bbae1a0572a79172f75ac003031
Latest deploy log https://app.netlify.com/sites/govuk-design-system-preview/deploys/671fbc20118328000830af18
Deploy Preview https://deploy-preview-4220--govuk-design-system-preview.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

hazalarpalikli commented 1 month ago

Thank you Owen! I noticed a minor issue that I wanted to bring to your attention. I realised I didn’t clearly indicate this in my designs: could we align the scroll with the hover state border? It creates a much cleaner and neater appearance. We are also using the same alignment in the current designs. Attaching designs below

Screenshot 2024-10-16 at 14 58 57

![Uploading Screenshot 2024-10-16 at 15.02.52.png…]()

selfthinker commented 1 month ago

I've not tested this yet but can do that tomorrow. But from a quick glance I noticed that the search icon now jumps between not having typed anything yet and starting to type.

animated gif showing what's described above

owenatgov commented 1 month ago

Chat about this with @hazalarpalikli off-github and unfortunately I haven't been able to move the scrollbar down to line up with the first item.

It's really hard to 'move' the scrollable area of an element whilst still maintaining just one element. I've tried a few things like a white box shadow or a border to emulate white space above the menu but the tricky part is always getting the :before fake border to leave the menu because of the same menu's overflow styling which we want to maintain. That's besides the fact that using something like a border or a box shadow to mock an element being taller than it actually is creates a lot of complexity and risk.

We could resolve this by using a menu 'wrapper' element with some top padding that would contain the ul, that way the ulcould still be scrollable whilst having some non-scrollable space above it. However in order to touch the runtime markup we'd need to change the accessible autocomplete, which means coordinating a release of the autocomplete which means this will get significantly delayed. I think it'd be good to move this upstream and solve the issue eventually but I suggest not for right now. I'm open to alternative suggestions for how to resolve this but I'd rather not spend too much more time on this specific issue for the moment.

In some more positive news, I've managed to resolve @selfthinker's bug. As far as I'm concerned this is now ready for accessibility reviewwhen @selfthinker has time, then it'll be ready for code review.

CharlotteDowns commented 1 month ago

I LOVE THIS!!

owenatgov commented 1 month ago

Update: I spotted another visual issue. Because the separator was positioned as a block element inside the menu, it disappears when you scroll down:

menu scrolled down with separator not visible

Good news though is that in solving this, I managed to solve @hazalarpalikli's scrollbar bug after all 🎉

The separator is now a pseudo element of the wrapper psotioned at the bottom of the input, only visible via :has. The use of :has means this will be an enhancement and the separator won't be available to browsers that don't support it, however the bottomless input styling is still present.

Speaking of that, I've also added some space between the first list item and the input using a top border that I adjust via forced-colors-mode. This is a little hacky but it does work from my own quick testing using firefox's light and dark colour themes and windows high contrast mode via AssistivLabs.

I've also done some very quick browser testing in Chrome and Safari macOS and its looking good there too.

selfthinker commented 4 weeks ago

I've tested this yesterday in Windows High Contrast Mode and Edge (three different colour schemes), Firefox browser settings (light and dark colour schemes) and the Chrome extension 'High Contrast'. I have uploaded screenshots of everything to this folder (which can only be viewed internally).

Generally, the colours look much better than when I tested the same change in the autocomplete component. Because the previous version didn't actually use the colours the user chose, and this version does. Although, that is not a result of this change, that was already the case before, but I thought it's worth highlighting anyway because we weren't happy with the colours on the other version.

The good news is that everything looks fine. The bad news is that the difference between having a dropdown and not having a dropdown is not as distinct in all of those tools I've tested, with the exception of the 'High Contrast' extension.

But I don't think that is generally fixable when using this kind of subtle design. So, I'm happy for this to get merged the way it is.

owenatgov commented 4 weeks ago

I'm gonna mark this for code review off of @selfthinker's comment. After that this can finally be merged ✨