Skip to content

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

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

Merged
matyasf merged 1 commit into from
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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/ui-buttons/src/BaseButton/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ class BaseButton extends Component<BaseButtonProps> {
tabIndex,
styles,
makeStyles,
withFocusOutline,
...props
} = this.props

Expand Down Expand Up @@ -307,6 +308,7 @@ class BaseButton extends Component<BaseButtonProps> {
focusRingBorderRadius={String(
(styles?.content as { borderRadius?: string | number })?.borderRadius
)}
withFocusOutline={withFocusOutline}
>
<span css={styles?.content}>{this.renderChildren()}</span>
</View>
Expand Down
12 changes: 10 additions & 2 deletions packages/ui-buttons/src/BaseButton/props.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,16 @@ type BaseButtonOwnProps = {

/**
* Specifies the tabindex of the `Button`.
*
*/
tabIndex?: number

/**
* Manually control if the `Button` should display a focus outline.
*
* When left `undefined` (which is the default) the focus outline will display
* if this component is focusable and receives focus.
*/
withFocusOutline?: boolean
}

type BaseButtonStyleProps = {
Expand Down Expand Up @@ -218,7 +225,8 @@ const propTypes: PropValidators<PropKeys> = {
onClick: PropTypes.func,
onKeyDown: PropTypes.func,
renderIcon: PropTypes.oneOfType([PropTypes.node, PropTypes.func]),
tabIndex: PropTypes.number
tabIndex: PropTypes.number,
withFocusOutline: PropTypes.bool
}

const allowedProps: AllowedPropKeys = [
Expand Down
22 changes: 17 additions & 5 deletions packages/ui-top-nav-bar/src/TopNavBar/TopNavBarItem/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ class TopNavBarItem extends Component<TopNavBarItemProps, TopNavBarItemState> {

this.state = {
isSubmenuOpen: false,
isPopoverOpen: false
isPopoverOpen: false,
isFocused: false
}
}

Expand Down Expand Up @@ -347,7 +348,8 @@ class TopNavBarItem extends Component<TopNavBarItemProps, TopNavBarItemState> {
renderSubmenu,
status: statusOriginal,
renderAvatar,
renderIcon
renderIcon,
withFocusOutline
} = this.props

let href = hrefOriginal
Expand Down Expand Up @@ -423,18 +425,28 @@ class TopNavBarItem extends Component<TopNavBarItemProps, TopNavBarItemState> {
onClick,
onMouseOver,
onMouseOut,
onFocus,
onBlur,
onFocus: createChainedFunction(onFocus, this.onFocus),
onBlur: createChainedFunction(onBlur, this.onBlur),
onKeyDown: createChainedFunction(onKeyDown, this.handleKeyDown),
onKeyUp,
renderIcon,
themeOverride: this.buttonThemeOverride,
elementRef: (e) => {
this.handleItemRef(e as HTMLButtonElement | HTMLLinkElement)
}
},
withFocusOutline:
withFocusOutline || this.hasOpenPopover || this.state.isFocused
Comment on lines +437 to +438
Copy link
Collaborator Author

@matyasf matyasf May 29, 2025 •
edited
Loading

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)

}
}

onFocus = () => {
this.setState({ isFocused: true })
}

onBlur = () => {
this.setState({ isFocused: false })
}

handleKeyDown: TopNavBarItemProps['onKeyDown'] = (e) => {
if (e.key === 'ArrowDown') {
if (
Expand Down
15 changes: 13 additions & 2 deletions packages/ui-top-nav-bar/src/TopNavBar/TopNavBarItem/props.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,14 @@ type TopNavBarItemOwnProps = {
* Should close the container menu component, if clicked on the option marked with this prop
*/
shouldCloseOnClick?: ShouldCloseOnClick

/**
* Manually control if this component should display a focus outline.
*
* When left `undefined` (which is the default) the focus outline will display
* if this component is focusable and receives focus or has an open popover.
*/
withFocusOutline?: boolean
}

type PropKeys = keyof TopNavBarItemOwnProps
Expand Down Expand Up @@ -281,6 +289,7 @@ type TopNavBarItemStyle = ComponentStyle<
type TopNavBarItemState = {
isSubmenuOpen: boolean
isPopoverOpen: boolean
isFocused: boolean
}

type TopNavBarItemStyleProps = {
Expand Down Expand Up @@ -316,7 +325,8 @@ const propTypes: PropValidators<PropKeys> = {
onKeyUp: PropTypes.func,
elementRef: PropTypes.func,
itemRef: PropTypes.func,
shouldCloseOnClick: PropTypes.oneOf(['auto', 'always', 'never'])
shouldCloseOnClick: PropTypes.oneOf(['auto', 'always', 'never']),
withFocusOutline: PropTypes.bool
}

const allowedProps: AllowedPropKeys = [
Expand All @@ -343,7 +353,8 @@ const allowedProps: AllowedPropKeys = [
'onKeyUp',
'elementRef',
'itemRef',
'shouldCloseOnClick'
'shouldCloseOnClick',
'withFocusOutline'
]

export type {
Expand Down
3 changes: 2 additions & 1 deletion packages/ui-top-nav-bar/src/TopNavBar/TopNavBarLayout/SmallViewportLayout/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,8 @@ class TopNavBarSmallViewportLayout extends Component<
tooltip: dropdownMenuToggleButtonTooltip,
themeOverride: { itemSpacing: '0.375rem' },
'aria-haspopup': 'menu',
'aria-expanded': isDropdownMenuOpen
'aria-expanded': isDropdownMenuOpen,
withFocusOutline: isDropdownMenuOpen ? true : undefined
}

const alternativeTitleIconProps = {
Expand Down
3 changes: 2 additions & 1 deletion packages/ui-view/src/View/props.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ type ViewOwnProps = {
*/
insetBlockEnd?: string
/**
* Manually control if the `View` should display a focus outline.<br/>
* Manually control if the `View` should display a focus outline.
*
* When left `undefined` (which is the default) the focus outline will display
* automatically if the `View` is focusable and receives focus.
*/
Expand Down
Loading