Global search - keyboard + hiding #1416
Conversation
anselmbradford
force-pushed
the
global_search_fixes
branch
from
48a446f
to
e688433
Compare
| @@ -267,6 +258,22 @@ | |||
| } ); | |||
| } | |||
|
|
|||
| .m-global-search { | |||
| // Tab trigger is used to capture press of the tab key so that | |||
| // global search can be collapsed when it hits this element. | |||
There was a problem hiding this comment.
Just so I'm clear, the user tabs through the entire menu, would normally tab to the next item after the menu, but instead hits this hidden tab-trigger, which has an event observer that in turn closes the menu?
There was a problem hiding this comment.
That is correct. I guess it's a little weird in that after the menu closes the focus jumps back up to the beginning of the page. Not sure if that's an issue as it's still all tab-accessible (e.g. after collapsing, continue tabbing through the page past the search button). Perhaps focus should jump back to the closed search button? I'll push a fix for this.
This approach is to avoid a lot of complicated logic on tabbing behavior. The search box needs to close after tabbing from the search button at desktop and mobile size, but needs to instead close after the last of the suggested topics links at tablet size. I believe once the bewd-side comes in, the suggested topics links will not be static. Therefore there'd need to be logic around adding and removing events at different breakpoints and on the bewd-side to identify the last link, if present.
There was a problem hiding this comment.
I like it, it's a smart work around to a wicked complicated set of interactions. Nice work!
|
Should there be an animation on desktop-ish sizes? It looks like it just jumps from closed to open and in reverse. |
|
ditto Jimmys comment about animating on desktop. |
This step is technically a WCAG failure, but I don't think your PR introduced it. Clicking on the menu, then clicking on the search icon shows no visible changes. When I click on the menu "x" both things are dismissed, making it seem like the search never appeared. It might be worth talking with UX about this interaction because I think it could be fixed a couple ways. The search box toggle works really well on Voiceover and through keyboard tabbing. |
Fixed in ca0e092
These will slide back and forth. Search from the right, mega menu from the left. I just tried to isolate the global search behavior here and haven't included the mega menu behavior. |
anselmbradford
force-pushed
the
global_search_fixes
branch
from
ca0e092
to
dd16fd5
Compare
| var _triggerSel = '.' + BASE_CLASS + '_trigger'; | ||
| var _triggerDom = _dom.querySelector( _triggerSel ); | ||
| var _flyoutMenu = new FlyoutMenu( _dom, | ||
| _triggerSel, |
There was a problem hiding this comment.
Probably better to place one new line.
|
Looks good. 👍 . It doesn't always animate out when tabbing on mobile, but maybe that can be addressed later. |
| markup: | | ||
| <h1> | ||
| <a href="#"> | ||
| <span class="u-invisible">Not shown</span> |
There was a problem hiding this comment.
Should this class be on the a? Cause that'll still be in the tab index.
Yeah, I have a poorly worded todo for that here. Looks like it's a reflow issue again. It may be worth trying an animation instead of a transition and see if there's a difference. |
|
👍 |
anselmbradford
force-pushed
the
global_search_fixes
branch
from
b23f666
to
c7e58eb
Compare
|
👍 |
anselmbradford
force-pushed
the
global_search_fixes
branch
from
c7e58eb
to
4a99b78
Compare
Global search - keyboard + hiding
Changes
Testing
gulp buildKeyboard:
Mouse:
Review
Screenshots
Todos