Add styling for site search input 'flowing' into dropdown #4220
Conversation
✅ You can preview this change here:
To edit notification comments on pull requests, go to your Netlify site configuration. |
owenatgov
force-pushed
the
site-search-visual-cue-when-dropdown
branch
4 times, most recently
from
67d59bd to
001dc84
Compare
|
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 
|
|
I've not tested this yet but can do that tomorrow.
|
owenatgov
force-pushed
the
site-search-visual-cue-when-dropdown
branch
from
001dc84 to
f40b221
Compare
|
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 We could resolve this by using a menu 'wrapper' element with some top padding that would contain the 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. |
|
I LOVE THIS!! |
|
Update: I spotted another visual issue. Because the separator was positioned as a block element inside the menu, it disappears when you scroll down: 
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 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 I've also done some very quick browser testing in Chrome and Safari macOS and its looking good there too. |
owenatgov
force-pushed
the
site-search-visual-cue-when-dropdown
branch
from
f40b221 to
65a9f5a
Compare
|
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'. 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. But I don't think that is generally fixable when using this kind of subtle design. |
|
I'm gonna mark this for code review off of @selfthinker's comment. After that this can finally be merged ✨ |
| // This is here on the wrapper element and absolute positioned under the input | ||
| // rather than on the menu because we can't move a pseudo element out of the | ||
| // menu eg: using minus margins or absolute positioning without breaking the | ||
| // menu's overflow conditions. |
There was a problem hiding this comment.
As a thought on this, was placing the pseudo-element on the menu and increasing the menu's negative top margin to 'move it out' something that was tried? Seems like it'd be a bit neater, though obviously, I don't know if that'd work here.
There was a problem hiding this comment.
I gave this a go locally to quickly try to remind myself and unforunately it doesn't work. The issue is that the menu's overflow conditions mean that any negitive margin artificial positioning causes the element to disappear because it's inside a parent, the menu, with overflow-x: hidden. This doesn't seem like it would cause problems since we're talking about content in the 'y' direction, but I think this is what's noted on the overflow-y MDN page:
If overflow-x is hidden, scroll, or auto and the overflow-y property is visible (default), the value will be implicitly computed as auto.
I tried messing with the overflow so that it would only manage overflow for the bottom of the element and not the top bt couldn't figure it out. I'm open to ideas.
There was a problem hiding this comment.
Ack, that's rather annoying. Something that visually looks to be part of the dropdown whilst actually being detached from it feels... brittle. Probably hasn't helped with readability of stuff either.
I don't have a better idea to offer though, so I'll give it a poke assuming this is the best route available to us.
There was a problem hiding this comment.
Could position: fixed be helpful to keep the separator within the <ul> and at its top? There are a couple of properties you can set on a parent element that make an object using position: fixed not be fixed relative to the page, but a closer containing block.
There was a problem hiding this comment.
I gave @romaricpascal's position: fixed idea a go and unfortuantely the overflow problem comes back. fixed with contain: layout as an example doesn't let the pseudo element escale the menu parent, so long as those overflow settings are in place. Same results for the other suggestions provided by MDN. I could resolve this by changing the scope to app-site-search__wrapper but that's basically what I'm doing now.
Unless there's a way to adjust the overflow settings without breaking the component I'm not sure there's a less weird solution. The actual answer IMO is to put a container around the ul which we can do upstream at a later date.
owenatgov
force-pushed
the
site-search-visual-cue-when-dropdown
branch
from
65a9f5a to
0b19c2a
Compare
| // provide some space between the list items highlight-able top edge and the | ||
| // bottom of the input. | ||
| border-top: govuk-colour("white") solid govuk-spacing(1); | ||
| box-shadow: rgba(govuk-colour("black"), 0.256863) 0 govuk-spacing(1) govuk-spacing(1); // stylelint-disable-line number-max-precision |
There was a problem hiding this comment.
May want to interpolate function too.
| box-shadow: rgba(govuk-colour("black"), 0.256863) 0 govuk-spacing(1) govuk-spacing(1); // stylelint-disable-line number-max-precision | |
| box-shadow: rgba(#{govuk-colour("black")}, 0.256863) 0 govuk-spacing(1) govuk-spacing(1); // stylelint-disable-line number-max-precision |
There was a problem hiding this comment.
So doing this one gives me an error:
sass.Exception [Error]: $color: #0b0c0c is not a color.
I don't understand why. #0b0c0c is a colour so 🤷🏻 I wonder if this is some weird string thing.
Means that users who are very zoomed in can see that there is something below the input to interact with
owenatgov
force-pushed
the
site-search-visual-cue-when-dropdown
branch
from
0b19c2a to
ee7ec34
Compare
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 #4015
Visual changes
Notes
This PR follows alphagov/accessible-autocomplete#753. See alphagov/accessible-autocomplete#753 (comment) for testing notes from that PR.
There's a little gap between the fake
:beforeborder 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:beforeelement 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.