Replace trip sort select with accessible dropdown #897
Conversation
binh-dam-ibigroup
assigned miles-grant-ibigroup and amy-corson-ibigroup and unassigned binh-dam-ibigroup
| /> | ||
| </span> | ||
| } | ||
| nav |
There was a problem hiding this comment.
Just remove the unused nav prop.
There was a problem hiding this comment.
Very clever re-use of components and great cleanup!
I have a few cosmetic notes before I dig into the code -- I'm sad to see the locale selector dropdown has lost its background on click. I would be sad to lose it but if it's not possible to keep then I think it's worth it based on how good the new dropdown looks!
The second thing is that because the dropdown is now non-native the leave feedback button that's sometimes to the left of it looks very out of place. Do you think it's worth creating a matching button class?
|
Fixed merge conflicts! I am not seeing any issues with the keyboard navigation on the dropdown, can you let me know if you still are? |
| const NavItemOnLargeScreens = styled(NavbarItem)` | ||
| display: block; | ||
| @media (max-width: 768px) { | ||
| display: none !important; | ||
| } | ||
| ` | ||
|
|
There was a problem hiding this comment.
Now that we control NavbarItem we can probably add this rule right to NavbarItem! This hack was only there because I couldn't touch bootstrap
There was a problem hiding this comment.
We're using NavbarItem for the log in/sign up button as well, which should stay visible at smaller screen sizes, so I think the custom rule is still necessary!
|
Thank you both for all the help with this one! |
Description:
Select to sort trip results updates page without warning (3.2.2 On Input). This PR repurposes the locale selector dropdown to provide a better approach to sorting in terms of accessibility.
PR Checklist: