Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 26 additions & 3 deletions packages/ui-drilldown/src/Drilldown/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -887,8 +887,20 @@ class Drilldown extends Component<DrilldownProps, DrilldownState> {
}

handleToggle = (event: React.UIEvent | React.FocusEvent, shown: boolean) => {
const { onToggle } = this.props

const { onToggle, trigger } = this.props

if (trigger && shown && this.currentPage) {
const actionLabel = callRenderProp(this.currentPage.renderActionLabel)
// Use action ID if exists, otherwise first non-action option's ID
const targetId = actionLabel
? this._headerActionId
: this.currentPage.children[0]?.props.id
setTimeout(() => {
this.setState({
highlightedOptionId: targetId
})
}, 10)
}
this.setState({ isShowingPopover: shown })

if (typeof onToggle === 'function') {
Expand Down Expand Up @@ -1481,7 +1493,7 @@ class Drilldown extends Component<DrilldownProps, DrilldownState> {
this.handleToggle(event, false)
}}
onShowContent={(event) => this.handleToggle(event, true)}
mountNode={mountNode}
mountNode={mountNode || this.ref}

@ToMESSKa ToMESSKa May 15, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This solution is based on mounting the DrillDown to the component (not to document.body), allowing us to navigate back from the first item to the trigger button itself.

@matyasf matyasf May 29, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should mention this in the release notes, it might break stuff for users

Comment thread
ToMESSKa marked this conversation as resolved.
Outdated
placement={placement}
withArrow={withArrow}
positionTarget={positionTarget}
Expand All @@ -1495,6 +1507,17 @@ class Drilldown extends Component<DrilldownProps, DrilldownState> {
onMouseOver={onMouseOver}
offsetX={offsetX}
offsetY={offsetY}
defaultFocusElement={() => {
if (!this.currentPage) return null
const actionLabel = callRenderProp(this.currentPage.renderActionLabel)
// Use action ID if exists, otherwise first non-action option's ID
const targetId = actionLabel
? this._headerActionId
: this.currentPage.children[0]?.props.id

if (!targetId) return null
return this._popover?._contentElement?.querySelector(`#${targetId}`)
}}
elementRef={(element) => {
// setting ref for "Popover" version, the popover root
// (if there is no trigger, we set it in handleDrilldownRef)
Expand Down
2 changes: 1 addition & 1 deletion packages/ui-drilldown/src/Drilldown/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ type DrilldownOwnProps = {

/**
* If a trigger is supplied, an element or a function returning an element
* to use as the mount node for the `<Drilldown />` (defaults to `document.body`)
* to use as the mount node for the `<Drilldown />` (defaults to the component itself)
*/
mountNode?: PositionMountNode

Expand Down
4 changes: 1 addition & 3 deletions packages/ui-top-nav-bar/src/TopNavBar/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -648,12 +648,10 @@ class PlaygroundExample extends React.Component {

{...item.submenu && {
renderSubmenu: this.generateSubmenu(item),
'aria-label': `Open for ${item.label} menu`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Réka said aria-label is unnecessary hin these places and advised to remove them

}}

{...item.customPopoverConfig && {
customPopoverConfig: item.customPopoverConfig,
'aria-label': `Open for ${item.label} menu`
}}

{...!item.submenu && !item.customPopoverConfig ? {
Expand Down Expand Up @@ -720,7 +718,7 @@ class PlaygroundExample extends React.Component {
<TopNavBar.Item
id="Info"
renderIcon={<IconQuestionLine />}
aria-label="Open for info menu"
aria-label="info menu"
renderSubmenu={this.generateSubmenu({
id: 'Info',
submenu: ['Contact', 'Map', 'Career'].map(item => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,9 @@ const generateStyle = (
alignItems: 'stretch',
justifyContent: 'space-between',
height: componentTheme.desktopHeight,
position: 'relative',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

these positions and overflows needed to be modified now that Drilldown is no longer mounted to the end of the document, otherwise the styling breaks

zIndex: componentTheme.desktopZIndex,
maxWidth: '100%',
overflow: 'hidden',
overflow: 'visible',
paddingInline: componentTheme.desktopInlinePadding,
paddingBlock: 0,
...(hasBrandBlock && {
Expand Down Expand Up @@ -113,7 +112,6 @@ const generateStyle = (
marginInline: componentTheme.desktopUserContainerInlineMargin,

...(hasUserSeparator && {
position: 'relative',
paddingInlineStart: componentTheme.desktopUserSeparatorGap,

'&::before': {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class TopNavBarSmallViewportLayout extends Component<
private readonly _inPlaceDialogId: string
private readonly _inPlaceDialogCloseButtonId: string
private readonly _separatorId: string
private _drilldownRef: Drilldown | null = null

private _raf: RequestAnimationFrameType[] = []

Expand Down Expand Up @@ -286,7 +287,44 @@ class TopNavBarSmallViewportLayout extends Component<
onDropdownMenuToggle(!isDropdownMenuOpen)
}

this.setState({ isDropdownMenuOpen: !isDropdownMenuOpen })
this.setState(
{ isDropdownMenuOpen: !this.state.isDropdownMenuOpen },
() => {
if (this.state.isDropdownMenuOpen) {
this.focusFirstAvailableItem()
}
}
)
}

focusFirstAvailableItem() {
const userChildren = Children.toArray(
this.props.renderUser?.props.children
) as ItemChild[]

// If option is a User, it preceeds other item, so focus that first
const targetId = userChildren[0]
? userChildren[0].props.id
: this.mappedMenuItemsOptions[0].optionData.id

setTimeout(() => {
const container = document.getElementById(this._trayContainerId)
const firstOption = container?.querySelector(
`[id="${targetId}"]`
) as HTMLSpanElement
firstOption?.focus()
if (this._drilldownRef) {
const drilldownRef = this._drilldownRef
setTimeout(() => {
if (drilldownRef) {
// highlight the option using Drilldown's handleOptionHighlight function
drilldownRef.handleOptionHighlight({} as React.SyntheticEvent, {
id: targetId
})
}
}, 10)
}
}, 10)
}

renderOptionContent: RenderOptionContent = (children, itemProps) => {
Expand Down Expand Up @@ -402,6 +440,9 @@ class TopNavBarSmallViewportLayout extends Component<
return (
<div css={styles?.menuTriggerContainer}>
{menuTrigger}
{!this.hasBreadcrumbBlock && !this.props.trayMountNode && (
<div css={styles?.trayContainer} id={this._trayContainerId} />
)}

{this.hasBrandBlock(renderBrand) && !alternativeTitle && (
<div css={styles?.brandContainer}>{renderBrand}</div>
Expand Down Expand Up @@ -442,6 +483,9 @@ class TopNavBarSmallViewportLayout extends Component<

return (
<Drilldown
ref={(el) => {
this._drilldownRef = el
}}
id={this._drilldownId}
rootPageId={this._menuId}
label={dropdownMenuLabel}
Expand Down Expand Up @@ -576,13 +620,7 @@ class TopNavBarSmallViewportLayout extends Component<
}

render() {
const {
trayMountNode,
navLabel,
renderActionItems,
renderBreadcrumb,
styles
} = this.props
const { navLabel, renderActionItems, renderBreadcrumb, styles } = this.props

return (
<nav
Expand All @@ -607,10 +645,6 @@ class TopNavBarSmallViewportLayout extends Component<

{!this.hasBreadcrumbBlock && this.renderInPlaceDialog()}

{!this.hasBreadcrumbBlock && !trayMountNode && (
<div css={styles?.trayContainer} id={this._trayContainerId} />
)}

Comment on lines 610 to 613

@ToMESSKa ToMESSKa May 20, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I moved this code after line 466 so that when I'm on the first item of the drill-down menu with SR and step back, the focus returns to the Drilldown's trigger, thereby exiting the DrillDown.

{!this.hasBreadcrumbBlock && this.renderDropdownMenuTray()}
</nav>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ const generateStyle = (
flexDirection: 'row',
alignItems: 'stretch',
// padding to prevent focus ring getting cropped by `overflow: hidden`
padding: '0 0.125rem'
padding: '0 0.125rem',
overflow: 'visible'
},
submenuOption: {
label: 'topNavBarMenuItems__submenuOption',
Expand Down
Loading