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.

Sign up for GitHub

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

Jump to bottom

fix: make tasks header navigable with the keyboard. #2313

Merged
merged 19 commits into from
May 30, 2023
Merged

fix: make tasks header navigable with the keyboard. #2313

merged 19 commits into from
May 30, 2023

Conversation

musale
Copy link
Contributor

@musale musale commented May 16, 2023

Closes #2251

PR Type

  • Bugfix

Description of the changes

  • Makes the assigned to me header navigable with the keyboard.
  • Changed the assigned to me dropdown menu to a fluentui menu

PR checklist

  • Project builds (yarn build) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)
  • All public APIs (classes, methods, etc) have been documented following the jsdoc syntax
  • Stories have been added and existing stories have been tested
  • Added appropriate documentation. Docs PR:
  • Accessibility tested and approved
  • License header has been added to all new source files (yarn setLicense)
  • Contains NO breaking changes

Other information

@ghost
Copy link

ghost commented May 16, 2023

Thank you for creating a Pull Request @musale.

This is a checklist for the PR reviewer(s) to complete before approving and merging this PR:

  • I have verified a documentation PR has been linked and is approved (or not applicable)
  • I have ran this PR locally and have tested the fix/feature
  • I have verified that stories have been added to storybook (or not applicable)
  • I have tested existing stories in storybook to verify no regression has occured
  • I have tested the solution in at least two browsers (Edge + 1 non-Chromium based browser)

@musale musale linked an issue May 16, 2023 that may be closed by this pull request
[MGTP-Samples-General-Localization]: 'Assigned to me' button under 'Localization' page is not accessible using Keyboard. #2251
Closed
@github-actions
Copy link

The updated storybook is available here

1 similar comment
@github-actions
Copy link

The updated storybook is available here

@github-actions
Copy link

The updated storybook is available here

@gavinbarron
Copy link
Member

gavinbarron commented May 16, 2023 •
edited by musale
Loading

@musale awesome stuff!

A few things I'd like for this to be even better.

  • Use a button instead of a link for the clickable header, this is an action not a navigation.
  • When a keyboard is used to open the menu move the focus to the first item in the menu.
  • When focus is within the menu if the user presses tab then the menu should be closed and the focus should move as expected to the next interactive item outside the menu
  • When focus is within the menu pressing escape closes the menu and returns focus to the header used to open the menu
  • When a menu item is selected focus returns to the header controlling that menu
  • (optional but preferred) When the menu is open a click outside the menu closes the menu

musale added 7 commits May 17, 2023 13:10
@musale musale requested a review from a team as a code owner May 17, 2023 12:45
@musale musale requested a review from gavinbarron May 17, 2023 12:45
@github-actions
Copy link

The updated storybook is available here

@gavinbarron
Copy link
Member

@musale for some reason the button is not opening the menu when I click in Storybook, the keyboard navigation works great though :)

@sebastienlevert
Copy link
Contributor

@vagpt and @KarunaNarang this item is ready for review.

@github-actions
Copy link

The updated storybook is available here

@musale musale mentioned this pull request May 24, 2023
Closed
@musale musale linked an issue May 25, 2023 that may be closed by this pull request
[MGTP-Components-Samples-General-Localization ]: ‘List items under Task group’, ‘Task bucket’, Assigned to’ buttons are not accessible using keyboard. #2344
Closed
@github-actions
Copy link

The updated storybook is available here

@github-actions
Copy link

The updated storybook is available here

gavinbarron
gavinbarron approved these changes May 30, 2023
View reviewed changes
Copy link
Member

@gavinbarron gavinbarron left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link

The updated storybook is available here

@gavinbarron gavinbarron enabled auto-merge (squash) May 30, 2023 18:24
@github-actions
Copy link

The updated storybook is available here

@gavinbarron gavinbarron merged commit 4747189 into next/fluentui May 30, 2023
@gavinbarron gavinbarron deleted the ac11y-2251 branch May 30, 2023 18:28
This was referenced Jul 17, 2023
Closed
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gavinbarron gavinbarron gavinbarron approved these changes

@sebastienlevert sebastienlevert Awaiting requested review from sebastienlevert

@Mnickii Mnickii Awaiting requested review from Mnickii

@vagpt vagpt Awaiting requested review from vagpt

Assignees
No one assigned
Labels
Build: Storybook
Projects
Graph Toolkit
Archived in project
Milestone
No milestone
3 participants
@musale @gavinbarron @sebastienlevert