Skip to content

fix(ui-top-nav-bar,ui-buttons): display a focus ring in TopNavBar if a button has a Popover open#1968

Merged
matyasf merged 1 commit intomasterfrom
fix_topnav_focus
Jun 2, 2025
Merged

fix(ui-top-nav-bar,ui-buttons): display a focus ring in TopNavBar if a button has a Popover open#1968
matyasf merged 1 commit intomasterfrom
fix_topnav_focus

Conversation

@matyasf
Copy link
Collaborator

@matyasf matyasf commented May 12, 2025

When a menu is active, it should be indicated as the “active” element for a11y reasons

To test: The desktop layout TopNavBar should show an outline around a button if it has some popover, like a submenu or a popup open. There should be no other change

Fixes part of INSTUI-4323

@matyasf matyasf self-assigned this May 12, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that when a button in the TopNavBar has an open popover, a focus ring is displayed for improved accessibility. Key changes include:

  • Updating the documentation for the focus outline control in ui-view.
  • Passing the withFocusOutline prop from TopNavBarItem to the underlying button component.
  • Adding the withFocusOutline prop to ui-buttons components for proper focus ring handling.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
packages/ui-view/src/View/props.ts Updated the JSDoc for the withFocusOutline prop
packages/ui-top-nav-bar/src/TopNavBar/TopNavBarItem/index.tsx Added withFocusOutline prop based on popover state
packages/ui-buttons/src/BaseButton/props.ts Introduced the withFocusOutline prop with documentation and prop types
packages/ui-buttons/src/BaseButton/index.tsx Destructured and passed the withFocusOutline prop to the underlying View component

@github-actions
Copy link

github-actions bot commented May 12, 2025

PR Preview Action v1.6.1
Preview removed because the pull request was closed.
2025-06-02 08:59 UTC

Copy link
Contributor

@ToMESSKa ToMESSKa left a comment

Choose a reason for hiding this comment

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

in the smallViewport version, the hambuger/X menu icon does not have a focus ring when the drilldown is open

@matyasf matyasf force-pushed the fix_topnav_focus branch from e453c9d to 449e0d9 Compare May 29, 2025 11:04
…a button has a Popover open

When a menu is active, it should be indicated as the “active” element for a11y reasons

Fixes part of INSTUI-4323
@matyasf matyasf force-pushed the fix_topnav_focus branch from 449e0d9 to 9b69f77 Compare May 29, 2025 13:28
Comment on lines +437 to +438
withFocusOutline:
withFocusOutline || this.hasOpenPopover || this.state.isFocused
Copy link
Collaborator Author

@matyasf matyasf May 29, 2025

Choose a reason for hiding this comment

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

This makes the TopNav items display a focus ring if their popover is open or its forced manually (this is needed for the smallViewport hamburger menu because that one does not use the customPopoverConfig prop of this class)

@matyasf
Copy link
Collaborator Author

matyasf commented May 29, 2025

in the smallViewport version, the hambuger/X menu icon does not have a focus ring when the drilldown is open

I've fixed this

@matyasf matyasf requested a review from ToMESSKa May 29, 2025 13:32
@matyasf matyasf merged commit 1a03763 into master Jun 2, 2025
12 checks passed
@matyasf matyasf deleted the fix_topnav_focus branch June 2, 2025 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants