Skip to content

fix(#3735): fix tooltip position in Work Side Menu and add to toggle button#3786

Open
bdfranck wants to merge 1 commit intodevfrom
benji/3735-fix-work-side-menu-tooltips
Open

fix(#3735): fix tooltip position in Work Side Menu and add to toggle button#3786
bdfranck wants to merge 1 commit intodevfrom
benji/3735-fix-work-side-menu-tooltips

Conversation

@bdfranck
Copy link
Copy Markdown
Collaborator

@bdfranck bdfranck commented Apr 8, 2026

This PR adds the following fixes to the Work Side Menu:

  • Adds tooltips to the Toggle button and the Search button in Docs for better usability.
  • Ensures consistent tooltip alignment, both horizontally and vertically, even when the menu is offset.
  • Adds a component token to allow dynamic setting of the menu height.

tooltip-fixed

@bdfranck bdfranck changed the title Benji/3735 fix work side menu tooltips fix(#3735): fix tooltip position in Work Side Menu and add to toggle button Apr 8, 2026
@bdfranck bdfranck requested a review from Copilot April 8, 2026 23:10

This comment was marked as resolved.

@bdfranck bdfranck force-pushed the benji/3735-fix-work-side-menu-tooltips branch from 979008d to d4d239d Compare April 9, 2026 14:46
// Menu-scoped keyboard navigation (only fires when focus is inside menu)
function handleMenuKeyDown(e: KeyboardEvent) {
switch (e?.key) {
case "ArrowDown":
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I forgot to remove this from a previous PR. It was causing errors so I removed it here.

@bdfranck bdfranck marked this pull request as ready for review April 9, 2026 15:24
Copy link
Copy Markdown
Collaborator

@twjeffery twjeffery left a comment

Choose a reason for hiding this comment

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

Looks great to me

Image

@Spark450 Spark450 requested a review from chrisolsen April 10, 2026 15:26
}, 300);
}

function showToggleTooltip() {
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 rename this handleToggleHover() instead of showToggleTooltip() as it may cause the confusion between showTooltip and showToggleTooltip.

Also we can shorten like this:

function handleToggleHover() {
if (open) return;

setTooltipPos("Expand menu", _toggleButtonEl);
showTooltip();
}

so under button below we can do like this:

<button ...
on:mouseenter={handleToggleHover}
on:mouseleave={hideTooltip}
on:focus={handleToggleHover}
on:blur={hideTooltip}

position: relative;
z-index: 101;
height: 100vh;
height: var(--goa-work-side-menu-height, 100vh);
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.

Do you have the token PR for this new variable? I don't see it on 2.2.2:

Image

<button
className="search-menu-button"
onClick={openSearch}
onMouseEnter={showSearchTooltip}
Copy link
Copy Markdown
Collaborator

@vanessatran-ddi vanessatran-ddi Apr 10, 2026

Choose a reason for hiding this comment

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

Question: was the reason for using a custom <button> here (instead of wrapping WorkSideMenuItem with a sentinel URL like /search) specifically to display the ⌘K keyboard shortcut as trailing content?

I asked because the doc site already has an established pattern for non-navigation items using WorkSideMenuItem + URL + wrapper onClick
For example: ComponentsSubMenu

If the ⌘K is the only blocker, we could drop it and use the badge like now instead, then we can add a story to follow up for trailingSlot on the WorkSideMenuItem.

If we can use WorkSideMenuItem instead of button here then we can have tooltip feature without having to add a customized function here.

cc @twjeffery How do you think?

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