Skip to content

fix(pds-tooltip): keep an open tooltip in step with its slotted content - #915

Merged
QuintonJason merged 14 commits into
mainfrom
fix/tooltip-sync-slotted-content
Oct 28, 2026
Merged

QuintonJason merged 14 commits into
mainfrom
fix/tooltip-sync-slotted-content

Conversation

@QuintonJason

@QuintonJason QuintonJason commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Description

pds-tooltip shows a copy of its content slot in a body-level portal, made when it opens. Its slot observer only watched the slot wrapper's direct children ({ childList: true, subtree: false }), so a change deeper in the content never reached an open tooltip: a Turbo Frame that finishes loading, a framework re-render (a spinner swapped for an iframe inside nested pds-boxes), text or attribute updates. The open tooltip stayed a frozen snapshot until it was closed and reopened.

Found building automation hover cards in kajabi-products (Kajabi/kajabi-products#59779, #59791, #59801): the cards have to load their content with the host rather than on hover, and a card opened mid-load keeps its spinner until it's hovered again.

Fix: widen the existing slot observer rather than adding one.

  • It watches the host's own slot wrapper at any depth (childList, subtree, attributes, characterData), and only while the tooltip is open: attached when the portal is created, disconnected when it's removed. The wrapper is matched among the host's direct children, so a nested pds-tooltip in the trigger slot isn't mistaken for it.
  • Changes are batched to one re-copy per animation frame, into the existing portal: the portal element, its id and the trigger's aria-describedby stay put (the old code tore the portal down and rebuilt it).
  • Equality guard: each re-copy is compared, with Stencil's own hydrated/scope classes stripped from its pds-* descendants first, against the last copy actually shown (cached as a string), not a fresh read of the live portal — a connected clone's own side effects (a nested light-DOM component re-running its own connect lifecycle) can change the portal's DOM independently of any real content change, and comparing against that live state would drift out of sync with it permanently. If the stripped copy matches, there's no swap and no reposition, so screen readers aren't re-announced and iframes don't reload.
  • No feedback loop: only the hidden slot wrapper is observed, never the portal. Still copies (cloneNode) rather than moving nodes, so the framework that owns the slotted nodes keeps them.

Tracking: DSS-420

Size budget

The CDN component chunk budget in .size-limit.json moves from 360 KB to 362 KB, sized for this PR and its sibling #917 (pds-select option sync), which needs the same headroom. main measures 359.74 KB gzipped; this PR now measures 360.42 KB (the hydration-class filter came back during review, after a prior pass had dropped it — see below) and #917 measures 361.02 KB on its own (about 361.5 KB with both). The size check counts each changed component twice (ESM and SystemJS chunks).

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update

How Has This Been Tested?

  • unit tests: observer attaches only when open, at any depth, on the host's own wrapper; content changed while open shows in the portal; two changes before a frame → one re-copy; the portal, its id and the trigger's aria-describedby survive a re-copy; unchanged content → no swap, no reposition (node identity kept); a pds-* descendant's hydrated-only class change is skipped, but the same class change on a plain element still re-copies (the strip is host-scoped, matching keepPineHydrated's own contract); an SVG descendant's class change is read correctly (not silently dropped via className's SVGAnimatedString); the portal's own DOM changing independently (e.g. a connected clone's own side effects) doesn't cause a permanent recopy loop, since the comparison is against the last copy shown, not a live re-read; a pending re-copy is cancelled on close and on removal; observer disconnects on close.
  • e2e tests: replacing nested slotted content while open updates the portal with a single copy (real MutationObserver).
  • accessibility tests
  • tested manually

Core spec suite 3469/3469 and e2e 890/890 pass locally. nx run @pine-ds/core:build regenerates nothing (no API change). ESLint: 0 errors (5 pre-existing warnings).

Gauntlet: code, security and design reviewers ran on this PR: 0 blockers. Their findings are fixed here: the equality guard, the host-scoped wrapper lookup, test isolation (globals restored, queued animation-frame mock) and the added cases above, and the docs advice below.

Docs: the tooltip docs now say an open tooltip follows its content, and advise against content that changes continuously (screen readers announce updates), against iframes or media in tooltip content (they reload when it changes), and against relying on ids inside it (it's copied).

Remaining trade-offs:

  • Content that genuinely changes every frame still re-copies once per frame.
  • A change to the content prop while the tooltip is open still doesn't re-sync. That's pre-existing and out of scope here.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes
  • Design has QA'ed and approved this PR

Note

Medium Risk
Changes tooltip DOM sync, MutationObserver lifecycle, and aria-live updates while open; behavior is heavily tested but affects a widely used overlay component.

Overview
Fixes pds-tooltip so an open tooltip’s portal copy follows nested slotted HTML (e.g. async loads, framework re-renders) instead of staying frozen until close/reopen.

Behavior change: slot watching moves to portal open only, with deep MutationObserver coverage on the host’s content wrapper (not the old shallow observer at load time). Updates re-copy into the existing portal (same element, id, and aria-describedby), batched once per animation frame. A dedupe step compares stripped HTML (ignoring Stencil hydration/scope classes on pds-* nodes via keep-hydrated) against the last shown copy, avoiding pointless swaps that would re-announce to screen readers or reload iframes/media.

Docs add guidance on live updates, duplicate ids, and risky tooltip content. CDN chunk size budget rises 360 KB → 362 KB. New unit and e2e coverage exercises sync, batching, hydration-only skips, and teardown.

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

@QuintonJason QuintonJason self-assigned this Oct 28, 2026
@netlify

netlify Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for pine-design-system ready!

Name Link
🔨 Latest commit b135c9c
🔍 Latest deploy log https://app.netlify.com/projects/pine-design-system/deploys/6ae2499a31d9777f77202256
😎 Deploy Preview https://deploy-preview-915--pine-design-system.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@QuintonJason QuintonJason added the ran-gauntlet Multi-agent review gauntlet has been run on this branch label Oct 28, 2026
@github-actions github-actions Bot added bug Something isn't working package: core Changes have been made to the Core package labels Oct 28, 2026
@QuintonJason
QuintonJason marked this pull request as ready for review October 28, 2026 15:26
@QuintonJason
QuintonJason requested a review from a team as a code owner October 28, 2026 15:26

@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.

Stale Bugbot comment from a previous run.

@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 using default effort 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 95ebc2c. Configure here.

@pixelflips pixelflips left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few small notes inline.

One on the description: it's a little out of date after 95ebc2c and 56808d5. It still says hydration-only mutations are skipped before a copy is built and that the hydration-class filter was dropped (both now handled by strippedHtml, after the copy), and it lists a mixed-batch unit test that isn't in the spec. The 360.18 KB figure was measured before the keep-hydrated import came back, so it may be worth re-measuring.

@QuintonJason

Copy link
Copy Markdown
Contributor Author

Updated the description's equality-guard and size-budget sections, and the testing bullet — they were stale after 95ebc2c/56808d5 (hydration-class filter came back, so it's no longer skipped pre-copy; and the mixed-batch test never existed in this form). Re-measured the CDN chunk budget for real this time: 360.42 KB (was quoting a pre-review 360.18 KB), still inside the 362 KB limit.

@QuintonJason
QuintonJason merged commit 44cb462 into main Oct 28, 2026
27 checks passed
@QuintonJason
QuintonJason deleted the fix/tooltip-sync-slotted-content branch October 28, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working package: core Changes have been made to the Core package ran-gauntlet Multi-agent review gauntlet has been run on this branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants