Skip to content

Add a for attribute to tooltip #116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
treeder wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add a for attribute to tooltip #116

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 57 additions & 6 deletions tooltip/tooltip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export class Tooltip extends LitElement {
headline: { type: String },
text: { type: String },
open: { type: Boolean, reflect: true },
for: { type: String },
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Choose a reason for hiding this comment

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

medium

The for property logic is currently only executed once during connectedCallback. If the for attribute is changed dynamically, the tooltip will not update its target or listeners. Additionally, if for is removed, the internal tooltip element will retain the position: fixed style applied by updatePosition. Consider handling these changes reactively in the updated lifecycle method to ensure the component stays in sync with its properties.

}

constructor() {
Expand All @@ -15,30 +16,79 @@ export class Tooltip extends LitElement {
this.text = ''
this.type = 'plain'
this.open = false
this.for = ''
this.showTimeout = null
this.hideTimeout = null
}

connectedCallback() {
super.connectedCallback()
this.addEventListener('mouseenter', this.handleMouseEnter)
this.addEventListener('mouseleave', this.handleMouseLeave)
this.addEventListener('focus', this.handleFocus, true)
this.addEventListener('blur', this.handleBlur, true)
this.targetElement = this

// Defer finding the external target to ensure it is rendered
requestAnimationFrame(() => {
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Choose a reason for hiding this comment

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

high

Using requestAnimationFrame to defer event listener attachment can lead to a memory leak. If the component is disconnected before the callback executes, listeners will be attached to the targetElement (which may be an external element) but will never be removed because disconnectedCallback has already run. Adding a check for this.isConnected ensures listeners are only attached if the component is still active.

    requestAnimationFrame(() => {
      if (!this.isConnected) return

if (this.for) {
const root = this.getRootNode()
const el = root.getElementById ? root.getElementById(this.for) : document.getElementById(this.for)
if (el) {
this.targetElement = el
}
}

this.targetElement.addEventListener('mouseenter', this.handleMouseEnter)
this.targetElement.addEventListener('mouseleave', this.handleMouseLeave)
this.targetElement.addEventListener('focus', this.handleFocus, true)
this.targetElement.addEventListener('blur', this.handleBlur, true)

// Also listen on the tooltip itself so users can interact with rich tooltips
if (this.targetElement !== this) {
this.addEventListener('mouseenter', this.handleMouseEnter)
this.addEventListener('mouseleave', this.handleMouseLeave)
}
})

window.addEventListener('scroll', this.handleScroll, true)
}

disconnectedCallback() {
super.disconnectedCallback()
if (this.targetElement) {
this.targetElement.removeEventListener('mouseenter', this.handleMouseEnter)
this.targetElement.removeEventListener('mouseleave', this.handleMouseLeave)
this.targetElement.removeEventListener('focus', this.handleFocus, true)
this.targetElement.removeEventListener('blur', this.handleBlur, true)
}
this.removeEventListener('mouseenter', this.handleMouseEnter)
this.removeEventListener('mouseleave', this.handleMouseLeave)
this.removeEventListener('focus', this.handleFocus, true)
this.removeEventListener('blur', this.handleBlur, true)
window.removeEventListener('scroll', this.handleScroll, true)
this.clearTimeouts()
}

handleScroll = () => {
if (this.open && this.for && this.targetElement !== this) {
this.open = false
this.clearTimeouts()
}
}

updatePosition() {
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Choose a reason for hiding this comment

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

medium

The tooltip position is only calculated when it is first opened. If the window is resized or the layout shifts while the tooltip is visible, it will become misaligned with the target element. Consider adding a resize event listener to the window while the tooltip is open to trigger updatePosition and keep it correctly anchored.

if (!this.for || this.targetElement === this) return

const tooltipElement = this.shadowRoot.querySelector('.md-tooltip')
if (!tooltipElement) return

const targetRect = this.targetElement.getBoundingClientRect()

tooltipElement.style.position = 'fixed'
tooltipElement.style.top = `${targetRect.bottom + 4}px`
tooltipElement.style.left = `${targetRect.left + (targetRect.width / 2)}px`
// Note: transform: translateX(-50%) from CSS will handle the horizontal centering
}

handleMouseEnter = () => {
this.clearTimeouts()
this.showTimeout = setTimeout(() => {
this.updatePosition()
this.open = true
}, 500)
}
Expand All @@ -52,6 +102,7 @@ export class Tooltip extends LitElement {

handleFocus = () => {
this.clearTimeouts()
this.updatePosition()
this.open = true
}

Expand Down