Skip to content

FE-1350: fix popover point positioning - #9189

Merged
alex-e-leon merged 11 commits into
mainfrom
FE-1350-fix-popover-point-positioning
Aug 12, 2026
Merged

FE-1350: fix popover point positioning#9189
alex-e-leon merged 11 commits into
mainfrom
FE-1350-fix-popover-point-positioning

Conversation

@alex-e-leon

Copy link
Copy Markdown
Contributor

🌟 What is the purpose of this PR?

  • Fixes popover positioning + tracking when position "point" is set
  • Adds autofocus: "never" prop to inputs + buttons now that ark-ui + zag have implemented it (Support data-no-autofocus in focusTrap or similar chakra-ui/zag#3161 (comment))
  • Prevents close buttons + tooltips from being autofocused in dialogs
  • Removes custom code in popover to handle focus out properly now that zag.js have fixed the issue
  • Splits Tooltip + BaseTooltip to allow for full tooltip UI customization.

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

  • are internal and do not require a docs change

@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
hash Ready Ready Preview Aug 12, 2026 9:03am
hashdotdesign-tokens Ready Ready Preview Aug 12, 2026 9:03am
petrinaut Ready Ready Preview Aug 12, 2026 9:03am

@cursor

cursor Bot commented Aug 10, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches focus management across many form/overlay components and rewrite popover positioning logic, so regressions could affect keyboard accessibility and overlay placement. No auth, security, or data-handling changes.

Overview
Fixes popover positionFromPoint positioning and live tracking, and improves overlay autofocus behavior after bumping @ark-ui/react / @zag-js.

Popover now keeps point and cross-axis gap values current via refs + animation-frame listeners, applies flip-aware main-axis gaps through offset with a RepositionOnGapChange nudge, and replaces the custom tab-focus proxy with zag’s proxyTabFocus. Also adds positionerClassName and a transform-origin for open/close animation.

Autofocus gains autoFocus="never" (via resolveAutoFocusPropsdata-no-autofocus) on buttons and form controls. Dialog/drawer/popover close buttons and header action buttons opt out so focus lands on real content instead of chrome.

Tooltip is split into an unstyled, exported BaseTooltip plus a thin styled Tooltip wrapper for custom tooltip UIs.

Reviewed by Cursor Bugbot for commit eff00bb. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions github-actions Bot added area/deps Relates to third-party dependencies (area) area/apps > hash* Affects HASH (a `hash-*` app) area/infra Relates to version control, CI, CD or IaC (area) area/libs Relates to first-party libraries/crates/packages (area) type/eng > frontend Owned by the @frontend team area/apps area/apps > hash.design Affects the `hash.design` design site (app) labels Aug 10, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4e4eca5. Configure here.

Comment thread libs/@hashintel/ds-components/src/components/Popover/popover.tsx

@lunelson lunelson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, although I don't quite get the "center" and "outside" position options

@alex-e-leon
alex-e-leon enabled auto-merge August 12, 2026 08:43
@alex-e-leon

Copy link
Copy Markdown
Contributor Author

Looks good, although I don't quite get the "center" and "outside" position options

Those 2 story examples are poorly labeled/named. The 2 prop options are actually:

// The preferred position of the popover - depending on the viewport, trigger and content another position may be chosen for better fit
position?: Position;

// Instead of positioning around the trigger, position from a specific point inside or outside the trigger where 0,0 is the top left of the trigger element
positionFromPoint?: { x: number; y: number };

And the center and outside examples are just test cases for positionFromPoint.

positionFromPoint was specifically added for attaching a popover to an element inside a canvas (which has no dom node to connect to), which I needed in graph-viz, and almost all the tables in hash-app currently are virtualized in canvas too.

@alex-e-leon

Copy link
Copy Markdown
Contributor Author

Also, in case I haven't said this before - it's worth noting that ladle examples are actually the UI tests for the DS rather than documentation, the goal being to render out specific edge cases for visual testing + regressions.

It also happens to have a lot of crossover with documenting the various features + props though, which is great while we don't have proper generated documentation yet - but it can mean that you'll sometimes see examples which are more for handling edge cases than real expected usage.

@alex-e-leon
alex-e-leon added this pull request to the merge queue Aug 12, 2026
Merged via the queue into main with commit 7d02529 Aug 12, 2026
50 checks passed
@alex-e-leon
alex-e-leon deleted the FE-1350-fix-popover-point-positioning branch August 12, 2026 09:37
@hash-release hash-release Bot mentioned this pull request Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/apps > hash.design Affects the `hash.design` design site (app) area/apps > hash* Affects HASH (a `hash-*` app) area/apps area/deps Relates to third-party dependencies (area) area/infra Relates to version control, CI, CD or IaC (area) area/libs Relates to first-party libraries/crates/packages (area) type/eng > frontend Owned by the @frontend team

Development

Successfully merging this pull request may close these issues.

3 participants