Skip to content

fix(tooltip): NO-JIRA add Shadow DOM support for DtTooltipDirective#1100

Merged
Patricio Sabogal (patosabogal) merged 4 commits intostagingfrom
fix/tooltip-directive-shadow-dom-support
Mar 6, 2026
Merged

fix(tooltip): NO-JIRA add Shadow DOM support for DtTooltipDirective#1100
Patricio Sabogal (patosabogal) merged 4 commits intostagingfrom
fix/tooltip-directive-shadow-dom-support

Conversation

@patosabogal
Copy link
Copy Markdown
Contributor

@patosabogal Patricio Sabogal (patosabogal) commented Mar 4, 2026

Summary

  • DtTooltipDirective and DtTooltip's externalAnchor computed property relied on document.body.querySelector() which cannot pierce Shadow DOM boundaries. This caused tippy.js to receive null as its target when used inside custom elements (defineCustomElement), resulting in the error: tippy() was passed null as its targets (first) argument.
  • Uses getRootNode() instead of document.body for externalAnchor queries in tooltip.vue, so lookups resolve within the shadow root when inside a custom element, and fall back to document in regular DOM.
  • Mounts the directive's tooltip app inside the element's root node instead of document.body, creating per-root-node instances so each Shadow DOM gets its own tooltip container.

Context

https://dialpad.com/app/messages/agxzfnViZXItdm9pY2VyGAsSC1RleHRNZXNzYWdlGIDAqKO36MALDA

Test plan

  • Verify tooltips still work in regular Vue apps (no Shadow DOM)
  • Verify v-dt-tooltip directive works inside defineCustomElement / Shadow DOM
  • Verify DtTooltip with externalAnchor prop works inside Shadow DOM
  • Run existing tooltip tests

DtTooltipDirective and DtTooltip's externalAnchor relied on
document.body.querySelector() which cannot pierce Shadow DOM boundaries.
This caused tippy.js to receive null targets when used inside custom
elements (defineCustomElement).

- Use getRootNode() instead of document.body for externalAnchor queries
  in tooltip.vue, so lookups resolve within the shadow root
- Mount the directive's tooltip app inside the element's root node
  instead of document.body, and create per-root-node instances so each
  Shadow DOM gets its own tooltip container
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 4, 2026

Please add either the visual-test-ready or no-visual-test label to this PR depending on whether you want to run visual tests or not.
It is recommended to run visual tests if your PR changes any UI. ‼️

… tooltip directive

Replace mutable tooltipInstance closure variable with per-anchor root
node lookup to fix incorrect tooltip instance resolution when multiple
shadow roots are present. Cache root node reference on anchor elements
during beforeMount so cleanup works correctly after DOM detachment.
@patosabogal Patricio Sabogal (patosabogal) changed the title fix(tooltip): add Shadow DOM support for DtTooltipDirective fix(tooltip): NO-JIRA: add Shadow DOM support for DtTooltipDirective Mar 4, 2026
@patosabogal Patricio Sabogal (patosabogal) changed the title fix(tooltip): NO-JIRA: add Shadow DOM support for DtTooltipDirective fix(tooltip): NO-JIRA add Shadow DOM support for DtTooltipDirective Mar 4, 2026
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.

LGTM

@braddialpad
Copy link
Copy Markdown
Contributor

Adding Noelle Levy (@noellelevydialpad) since this was originally your change

@noellelevydialpad
Copy link
Copy Markdown
Contributor

Why do we need to render the tooltips in the same shadow dom as the anchor? why not keep it a singleton?

@braddialpad
Copy link
Copy Markdown
Contributor

Why do we need to render the tooltips in the same shadow dom as the anchor? why not keep it a singleton?

I think this is probably the way to go. This change feels overly complex

Comment on lines +318 to +320
return this.externalAnchor
? (this.$el.getRootNode() || document.body).querySelector(this.externalAnchor)
: getAnchor(this.$refs.anchor);
Copy link
Copy Markdown
Contributor

@braddialpad Brad Paugh (braddialpad) Mar 5, 2026

Choose a reason for hiding this comment

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

I think what we need to to do here is stop the use of querySelector as it doesn't penetrate shadowDom. Instead let's create a new prop called externalAnchorElement and instead of being a selector this would be a reference to an actual element. This way we don't have to use querySelector. We'll have to keep the externalAnchor prop for now so we don't cause a breaking change, but we can get rid of it in the next major release of dialtone.

Lets see if we can make this work with no changes to the prior version of tooltip.js, other than making it use this new prop. I think it is still okay if the tooltip renders at the document.body rather than the shadowRoot body.

…Element prop

Add externalAnchorElement prop to DtTooltip that accepts an HTMLElement
directly, bypassing querySelector which cannot pierce Shadow DOM boundaries.
Revert the per-shadow-root tooltip app approach in favor of keeping the
singleton pattern. The directive now passes the anchor element reference
directly instead of using a CSS selector string.
});

tooltipConfig.anchorElement = anchor;
anchor.setAttribute('data-dt-tooltip-id', tooltipId);
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.

I believe this is now unused.

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.

It's no longer used as a CSS selector for querySelector, but the attribute is still used for internal bookkeeping:

  • Line 79 (unmounted): reads the ID to call removeTooltip when the element is destroyed
  • Lines 85/91 (setupTooltip): reads the ID to check if this anchor already has a tooltip (update vs create) and to reuse the same ID

Removing it would break cleanup on unmount and tooltip updates.

Copy link
Copy Markdown
Contributor

@braddialpad Brad Paugh (braddialpad) left a comment

Choose a reason for hiding this comment

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

This looks good, thanks!

@braddialpad
Copy link
Copy Markdown
Contributor

Have we tested to see if this still solves the original issue?

getCurrentInstance() returns Vue's internal instance object. Calling
methods through instance.ctx does not properly trigger reactivity in
the singleton app, preventing DtTooltip components from rendering.

Store `this` (the component proxy) instead, which ensures method calls
go through Vue's reactive proxy and trigger re-renders.
created () {
globalThis.__DtTooltipDirectiveAppInstance = getCurrentInstance();
mounted () {
globalThis.__DtTooltipDirectiveApp = this;
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.

Previously this used getCurrentInstance() which returns Vue's internal instance object. Calling methods through instance.ctx (e.g. instance.ctx.addOrUpdateTooltip(...)) does modify the reactive tooltips array, but it does not trigger the singleton app's render function to re-run — meaning the DtTooltip components were never actually created.

Storing this (the component proxy) instead ensures that method calls go through Vue's public reactive proxy. When the proxy's methods mutate this.tooltips, Vue's reactivity system properly tracks the dependency and queues a re-render of the singleton app, which then creates the DtTooltip instances for each registered tooltip.

This was verified by testing in digital-channels inside a Shadow DOM custom element — the directive tooltips only started rendering after this change.

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.

sounds good 👍

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2026

✔️ Deploy previews ready!
😎 Dialtone documentation preview: https://dialtone.dialpad.com/deploy-previews/pr-1100/
😎 Dialtone-vue preview: https://dialtone.dialpad.com/vue/deploy-previews/pr-1100/

@patosabogal Patricio Sabogal (patosabogal) merged commit 121ffa2 into staging Mar 6, 2026
18 checks passed
@patosabogal Patricio Sabogal (patosabogal) deleted the fix/tooltip-directive-shadow-dom-support branch March 6, 2026 21:19
Brad Paugh (braddialpad) pushed a commit that referenced this pull request Mar 6, 2026
# [3.211.0](dialtone-vue/v3.210.0...dialtone-vue/v3.211.0) (2026-03-06)

### Bug Fixes

* **Combobox:** DLT-3057 combobox errors when hovering over dt-dropdown-separator ([#1097](#1097)) ([4798e0e](4798e0e))
* **Tooltip:** NO-JIRA add Shadow DOM support for DtTooltipDirective ([#1100](#1100)) ([121ffa2](121ffa2))

### Code Refactoring

* **Progress Circle:** NO-JIRA update ai gradient with css variables and fewer stops ([#1105](#1105)) ([8dd19ea](8dd19ea))

### Features

* **Editor:** NO-JIRA added props allowBackgroundColor & allowLineHeight ([#1103](#1103)) ([7c90308](7c90308))
* **Loader,progress Circle:** NO-JIRA polish DtProgressCircle visual and align DtLoader visual with DtProgressCircle ([#1102](#1102)) ([7b9e9b6](7b9e9b6))
* **Progress Circle:** DLT-2983 add progress-circle component ([#1098](#1098)) ([c0f0e40](c0f0e40))

### Reverts

* NO-JIRA revert progress circle polish commits for re-PR ([#1101](#1101)) ([a212c41](a212c41))
Brad Paugh (braddialpad) pushed a commit that referenced this pull request Mar 6, 2026
# [9.167.0](dialtone/v9.166.0...dialtone/v9.167.0) (2026-03-06)

### Bug Fixes

* **Combobox:** DLT-3057 combobox errors when hovering over dt-dropdown-separator ([#1097](#1097)) ([4798e0e](4798e0e))
* **Tooltip:** NO-JIRA add Shadow DOM support for DtTooltipDirective ([#1100](#1100)) ([121ffa2](121ffa2))

### Code Refactoring

* **Progress Circle:** NO-JIRA update ai gradient with css variables and fewer stops ([#1105](#1105)) ([8dd19ea](8dd19ea))

### Features

* **Editor:** NO-JIRA added props allowBackgroundColor & allowLineHeight ([#1103](#1103)) ([7c90308](7c90308))
* **Loader,progress Circle:** NO-JIRA polish DtProgressCircle visual and align DtLoader visual with DtProgressCircle ([#1102](#1102)) ([7b9e9b6](7b9e9b6))
* **Progress Circle:** DLT-2983 add progress-circle component ([#1098](#1098)) ([c0f0e40](c0f0e40))

### Reverts

* NO-JIRA revert progress circle polish commits for re-PR ([#1101](#1101)) ([a212c41](a212c41))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-visual-test Add this tag when the PR does not need visual testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants