Add alt-S to navigate between search regions #2189
Conversation
RoyEJohnson
changed the title
RoyEJohnson
force-pushed
the
search-navigation-issues
branch
from
bea6381 to
21fa234
Compare
RoyEJohnson
force-pushed
the
search-navigation-issues
branch
5 times, most recently
from
46e1eb7 to
55a52bf
Compare
|
|
||
| scrollTo(target); |
There was a problem hiding this comment.
Is this scrollTo() just gone now? I noticed it was gone from the tests too.
There was a problem hiding this comment.
Yes. focus will scroll an element into view unless you specify that it shouldn't.
RoyEJohnson
force-pushed
the
search-navigation-issues
branch
from
f0e562b to
d73dd2e
Compare
| ].map((q) => document?.querySelector(q)); | ||
|
|
||
| // Determine which region we are in (if any) | ||
| const currentSectionIndex = targets.findIndex((el) => el?.contains(document?.activeElement!)); |
There was a problem hiding this comment.
I'd do something like document?.activeElement && el?.contains(document.activeElement) because activeElement can be null and idk if contains likes nulls or undefineds.
There was a problem hiding this comment.
null is ok for contains. activeElement can be undefined, but there's no good way to make it so. In general, it will be set to body if you blur it from anything else.
I changed document?activeElement! to document?activeElement ?? null but could not write a test to hit the undefined branch.
Add documentation for Alt+S
Malar-Natarajan
temporarily deployed
to
rex-web-search-navigati-wby8aa
DISCO-90