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

feat(TabBar): accessibility improvements for TabBar & Tab #1468

Merged
merged 3 commits into from
Jul 3, 2024
Merged

feat(TabBar): accessibility improvements for TabBar & Tab #1468

merged 3 commits into from
Jul 3, 2024

Conversation

alaa-yahia
Copy link
Member

@alaa-yahia alaa-yahia commented Mar 26, 2024 •
edited
Loading

Implements LIBS-557

Description

  • TabBar has role="tablist"

  • Tab has role="tab"

  • selected prop maps to aria-selected

  • disabled prop maps to aria-disabled

  • TabBar acts as a composite component, a user can tab to TabBar

  • A user can use left/right arrow keys to move between Tab components.

  • Pressing the left/right arrow keys will cycle back to the first or last tab, ensuring continuous navigation.

  • Moving to a Tab activates it.

  • Tab components with overflowing labels displays a Tooltip with full label on hover and focus.

Known issues

  • When pressing the Tab key the focus go to the first Tab component regardless of the selected prop.

  • Tabs with disabled prop can be tabbed into.

Checklist

  • API docs are generated
  • Tests were added
  • Storybook demos were added

@alaa-yahia alaa-yahia requested a review from kabaros March 26, 2024 09:11
@alaa-yahia alaa-yahia requested a review from a team as a code owner March 26, 2024 09:11
@alaa-yahia alaa-yahia changed the base branch from alpha to master March 26, 2024 09:19
@alaa-yahia alaa-yahia changed the title feat(TabBar | Tab): accessibility improvements for TabBar & Tab feat(TabBar): accessibility improvements for TabBar & Tab Mar 26, 2024
kabaros
kabaros requested changes Apr 2, 2024
View reviewed changes
Copy link
Collaborator

@kabaros kabaros left a comment

Choose a reason for hiding this comment

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

the code looks good - could you add tests to cover the new scenarios. You can use add e2e if you can, but it might be easier to add unit tests for these new scenarios.

Also add a storybook story for the overflow scenario.

@alaa-yahia
Copy link
Member Author

the code looks good - could you add tests to cover the new scenarios. You can use add e2e if you can, but it might be easier to add unit tests for these new scenarios.

Also add a storybook story for the overflow scenario.

The storybook story is already included here, Do you suggest to add a separate one for the overflow ?
For the tests, there wasn't a unit test in the folder probably because the consumer is responsible for managing the state not the TabBar component. Should I include an example component and use it in the unit test?

@kabaros
Copy link
Collaborator

kabaros commented Apr 3, 2024

Do you suggest to add a separate one for the overflow ?

Yes, I mean to add a story for the overflow scenario

Should I include an example component and use it in the unit test?
Yes, you'll need to do that .. I don't know if it needs to manage the state (or just pass down tabs to it), but basically a component to be able the new logic you added for handling keys, so you render a tab bar, and trigger key events to make sure the right tabs have the focus.

@alaa-yahia
Copy link
Member Author

alaa-yahia commented Apr 3, 2024 •
edited
Loading

Do you suggest to add a separate one for the overflow ?

Yes, I mean to add a story for the overflow scenario

Should I include an example component and use it in the unit test?
Yes, you'll need to do that .. I don't know if it needs to manage the state (or just pass down tabs to it), but basically a component to be able the new logic you added for handling keys, so you render a tab bar, and trigger key events to make sure the right tabs have the focus.

What I mean the text overflow story is included here:
image

There isn't a test for handling the main functionality "moving between Tabs" because it is managed by the consumer component not the TabBar component.

@kabaros
Copy link
Collaborator

kabaros commented Apr 3, 2024 •
edited
Loading

What I mean the text overflow story is included here:

ah ok .. yeah then that's fine. No need to add anything.

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Jun 14, 2024 •
edited
Loading

🚀 Deployed on https://pr-1468--dhis2-ui.netlify.app

@dhis2-bot dhis2-bot temporarily deployed to netlify June 14, 2024 19:53 Inactive
@alaa-yahia alaa-yahia force-pushed the LIBS-557-tabbar-tab-accessibility-improvements branch from aa28c70 to 4841273 Compare June 15, 2024 01:22
@dhis2-bot dhis2-bot temporarily deployed to netlify June 15, 2024 01:26 Inactive
@dhis2 dhis2 deleted a comment from cypress bot Jun 15, 2024
@kabaros kabaros merged commit b095f5a into master Jul 3, 2024
16 checks passed
@kabaros kabaros deleted the LIBS-557-tabbar-tab-accessibility-improvements branch July 3, 2024 09:47
dhis2-bot added a commit that referenced this pull request Jul 3, 2024
@dhis2-bot
chore(release): cut 9.10.0 [skip release]
975764f 
# [9.10.0](v9.9.1...v9.10.0) (2024-07-03)

### Features

* **menu:** memoise menu's childrenToRender function and add valid role checks ([#1539](#1539)) ([bddbdae](bddbdae))
* **TabBar:** accessibility improvements for TabBar & Tab ([#1468](#1468)) ([b095f5a](b095f5a))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 9.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kabaros kabaros kabaros approved these changes

Assignees
No one assigned
Labels
accessibility released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@alaa-yahia @kabaros @dhis2-bot