Skip to content

Conversation

@franknoirot
Copy link
Contributor

@franknoirot franknoirot commented Dec 30, 2025

Following the MDN docs on Using the Popover API. Positioning non-modal popovers basically requires CSS Anchor Positioning, which isn't supported in Firefox yet, so I've also included this polyfill provided by Oddbird.

In future, we should learn to wield position-try-fallbacks to get the exact "never overflow" behavior we want from our Tooltips and popover menus. I figured this was a good first step without messing with too much. It's comical how long it took me to figure out these APIs, there is a crazy dearth of useful guidance on what's needed and what's not.

TODOs

  1. This code does not handle focus and blur well when the Tooltip is applied as a sibling to the focusable element, not a child of it. I believe some of our E2E tests rely on the "accessible name" of several buttons that would be changed by putting the Tooltip within them, but I'm not positive. Adding mouse events to any DOM element is fine, but only some elements are focusable. As such, we should get the closest focusable element and if that exists attach the focus and blur event listeners. Lord I can't wait for the interestinvoker API.

Before

Screenshot 2025-12-30 at 3 44 22 AM

After

Screenshot 2025-12-30 at 3 37 48 AM

Also I think this will unblock me making the modeling toolbar scrollable, which was stuck because I needed overflow-visible for the tooltips to render there, so that's neat.

With a polyfill for anchor positioning in CSS for Firefox et al
@franknoirot franknoirot requested a review from a team as a code owner December 30, 2025 08:49
@vercel
Copy link

vercel bot commented Dec 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
modeling-app Ready Ready Preview, Comment Dec 30, 2025 10:58pm

Comment on lines 109 to 110
inset-inline-start: anchor(50%);
inset-block-end: anchor(end);
Copy link
Contributor

Choose a reason for hiding this comment

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

Incorrect anchor positioning for top tooltip. anchor(end) refers to the bottom edge of the anchor element, which would position the tooltip's bottom edge at the anchor's bottom (overlapping/below the anchor). For a top tooltip, it should be:

inset-block-end: anchor(start);

This positions the tooltip's bottom edge at the anchor's top edge, placing it above the anchor.

Suggested change
inset-inline-start: anchor(50%);
inset-block-end: anchor(end);
inset-inline-start: anchor(50%);
inset-block-end: anchor(start);

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.


.tooltipWrapper.bottom-left {
inset-block-start: 100%;
inset-block-start: anchor(start);
Copy link
Contributor

Choose a reason for hiding this comment

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

The bottom-left tooltip positioning is incorrect. It uses anchor(start) which positions the tooltip's top edge at the anchor's top edge, causing it to overlap the parent element instead of appearing below it.

The old code was inset-block-start: 100% (position tooltip below parent), so the new code should use anchor(end) to position at the bottom edge of the anchor:

.tooltipWrapper.bottom-left {
  inset-block-start: anchor(end);
}

Note that the .bottom class correctly uses anchor(end) on line 133, making this inconsistency a clear bug.

Suggested change
inset-block-start: anchor(start);
inset-block-start: anchor(end);

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nah I understand the miscommunication here. These are poorly named but this recreates the existing behavior

Copy link
Contributor

Choose a reason for hiding this comment

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

Are these snapshot changes expected? It looks like before the sidebar tooltips were visible and now they are not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No something just changed and broke

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well yes, the snapshot changes are expected I think, because the tooltips go away when you click the pane button now, but there was an issue making nothing show up fixed here

Copy link
Contributor

@jacebrowning jacebrowning Dec 30, 2025

Choose a reason for hiding this comment

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

I'm also noticing that the position of the Feature Tree tooltips changed, comparing staging to the preview:

Screenshot 2025-12-30 at 11 29 23 AM

Screenshot 2025-12-30 at 11 29 31 AM

Is that expected? I personally prefer the old behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No thank is not expected, thank you! I must have messed up one of the definitions. When you say you prefer the old behavior do you mean the feature tree tooltip position only?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I'm only referring to the Feature Tree. Everything else I tested in the Vercel preview looked the same or better.

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.

3 participants