Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#12530] Allow header dropdown to be opened with enter key #12542

Merged

Conversation

domoberzin
Copy link
Contributor

@domoberzin domoberzin commented Aug 1, 2023

Fixes #12530

Outline of Solution
The current dropdown toggle on the header is created using <a> tags, so currently, the dropdown can actually be toggled using the arrow down key.

This PR changes those <a> tags to use <button>tags also with added classes to reduce any change in the visual appearance. This change will now allow the dropdown to be toggled with the Enter key instead of the arrow key.

This PR also allows tabbing to the Log Out button and fixes #12531 by adding tabindex=0 to the element

Copy link
Contributor

@weiquu weiquu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@weiquu weiquu requested a review from jasonqiu212 August 2, 2023 08:14
@weiquu weiquu added the s.FinalReview The PR is ready for final review label Aug 2, 2023
Copy link
Contributor

@jasonqiu212 jasonqiu212 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes! Just one small nit pick - The buttons seem to flash blue when clicking on it, which I think is different from the original behavior. Is it possible to disable this behavior as well?

Screen.Recording.2023-08-02.at.8.00.03.PM.mov

src/web/app/page.component.html Outdated Show resolved Hide resolved
@domoberzin
Copy link
Contributor Author

Thanks for the changes! Just one small nit pick - The buttons seem to flash blue when clicking on it, which I think is different from the original behavior. Is it possible to disable this behavior as well?

Screen.Recording.2023-08-02.at.8.00.03.PM.mov

@jasonqiu212 Updated with the behaviour in the video below, which i believe should be similar to the original behaviour, do let me know if any other changes are needed

Screen.Recording.2023-08-03.at.4.35.42.PM.mov

@weiquu weiquu requested a review from jasonqiu212 August 3, 2023 11:54
Copy link
Contributor

@jasonqiu212 jasonqiu212 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jasonqiu212 jasonqiu212 merged commit 5bc54fc into TEAMMATES:master Aug 4, 2023
9 checks passed
@samuelfangjw samuelfangjw added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging c.Feature User-facing feature; can be new feature or enhancement to existing feature and removed s.FinalReview The PR is ready for final review labels Aug 21, 2023
@samuelfangjw samuelfangjw added this to the V8.29.0 milestone Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Feature User-facing feature; can be new feature or enhancement to existing feature s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Header: "Log Out" button cannot be tabbed over Header: unable to expand dropdown with keyboard commands
4 participants