Skip to content

Fix custom tooltip showing "null" on sliders with child elements#5580

Closed
pReya wants to merge 1 commit intowled:mainfrom
pReya:fix/null-tooltip-slider
Closed

Fix custom tooltip showing "null" on sliders with child elements#5580
pReya wants to merge 1 commit intowled:mainfrom
pReya:fix/null-tooltip-slider

Conversation

@pReya
Copy link
Copy Markdown

@pReya pReya commented May 7, 2026

closes #5578

Summary

  • Switch the custom tooltip handler in index.js from pointerover/pointerout to pointerenter/pointerleave so it fires exactly once per real entry/exit.

Problem

The custom tooltip implementation at wled00/data/index.js saves an element's title attribute to data-title on hover, removes title (to suppress the native browser tooltip), and restores it from data-title on exit.

Because pointerover bubbles, the handler fired again whenever the cursor crossed onto a child element (e.g. moving from the sliderwrap div onto its or .sliderdisplay child). On that second invocation getAttribute("title") returned null (it had already been removed), and setAttribute("data-title", null) stored the literal string "null". Subsequent hovers then displayed a tooltip reading "null".

This was reproducible on the Value/Brightness slider on the Colors tab, and could affect any element with a title attribute that contains child nodes the cursor can land on.

Fix

pointerenter/pointerleave do not bubble — they fire only on the actual entry to and exit from the listening element, regardless of internal cursor movement. This makes the handler idempotent for a single hover and matches the intended semantics.

AI Disclaimer: Claude Code was used to find and fix this bug and create this PR description. However it's not "vibe coded" as I understand the code completely and have reviewed and tested it manually.

Summary by CodeRabbit

  • Bug Fixes
    • Improved tooltip event handling to prevent unintended tooltip triggers and dismissals in edge cases.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 380c0fe9-fbc3-40cb-a4cc-f1adf81dd8f1

📥 Commits

Reviewing files that changed from the base of the PR and between f82ab3c and fdd47a7.

📒 Files selected for processing (1)
  • wled00/data/index.js

Walkthrough

Tooltip event handlers in the UI are updated to use pointerenter and pointerleave instead of pointerover and pointerout. The tooltip creation, positioning, and title restoration logic remains unchanged.

Changes

Tooltip Event Handler Refinement

Layer / File(s) Summary
Tooltip Event Handlers
wled00/data/index.js
Tooltip show event listener switches from pointerover to pointerenter (line 3184). Tooltip hide/cleanup listener switches from pointerout to pointerleave with title restoration logic preserved (lines 3209–3212).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the fix: changing event handlers to prevent 'null' tooltips on sliders with child elements.
Linked Issues check ✅ Passed The code change addresses the root cause of issue #5578 by switching from pointerover/pointerout to pointerenter/pointerleave events to prevent storing 'null' in tooltips.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the tooltip issue reported in #5578; no unrelated modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pReya pReya force-pushed the fix/null-tooltip-slider branch from f82ab3c to b6335d3 Compare May 7, 2026 21:53
@pReya pReya force-pushed the fix/null-tooltip-slider branch from b6335d3 to fdd47a7 Compare May 7, 2026 21:54
@DedeHai
Copy link
Copy Markdown
Collaborator

DedeHai commented May 8, 2026

thanks for spotting and fixing this. The original code works for me on chrome and safari. On what setup did it not work?

@pReya
Copy link
Copy Markdown
Author

pReya commented May 8, 2026

thanks for spotting and fixing this. The original code works for me on chrome and safari. On what setup did it not work?

The original bug appeared in Firefox for me.

@DedeHai
Copy link
Copy Markdown
Collaborator

DedeHai commented May 8, 2026

Not seeing it in firefox either...

@DedeHai
Copy link
Copy Markdown
Collaborator

DedeHai commented May 8, 2026

Looks like a good update in general, I can't judge if it would have any negative side effects. Maybe the rabbit can dig a little.

@DedeHai
Copy link
Copy Markdown
Collaborator

DedeHai commented May 8, 2026

@coderabbitai does this change have any unforseen consequences? Please evaluate for all major browsers and operating systems.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@DedeHai: Great question — here's a thorough evaluation of the change from pointerover/pointeroutpointerenter/pointerleave across browsers and input types.


✅ Browser Support

Both pointerenter and pointerleave are supported in all major engines — Chrome, Firefox, Safari, and Edge (including IE 11+). There is no compatibility concern here.


🔍 Behavioral Difference (the core of this fix)

pointerover events have the same problems as mouseover: if the target element has child elements, pointerout and pointerover fire as the pointer moves over the boundaries of those child elements, not just the target element itself. pointerenter and pointerleave are generally more sensible because they are not affected by moving into child elements.

This is precisely what caused the "null" bug. The old pointerover handler would re-fire when hovering into the slider's child (input or .sliderdisplay), at which point getAttribute("title") returned null (since the title had already been removed), and null got stored as the literal string "null".

pointerleave "does not bubble and MUST not be fired until the pointing device has left the boundaries of the element and the boundaries of all of its descendants." This makes the new behavior idempotent — the tooltip is shown exactly once per hover, and hidden exactly once on exit.


🖱️ Mouse (Desktop) — Chrome, Firefox, Safari, Edge

No regressions. The tooltip will show on hover and dismiss on exit, firing exactly once each way. This is the intended behavior and matches what a native title tooltip would do.


👆 Touch (Mobile/Tablet)

For touchscreen browsers that allow direct manipulation, an implicit pointer capture is called on the element when a pointerdown event triggers, and pointerover, pointerenter, pointerleave, and pointerout will not fire as long as this capture is set.

This means on touch devices, tooltips were already effectively non-functional in both old and new code during a touch-drag interaction (e.g., sliding the slider). This is unchanged behavior — no regression.


🖊️ Pen / Stylus

The pointerleave event fires when a pointing device moves out of the hit test boundaries of an element; for pen devices, it fires when the stylus leaves the hover range detectable by the digitizer. This is equivalent to the old pointerout behavior, with the added correctness of not misfiring on child element boundaries.


⚠️ One Edge Case (Minor, Unlikely to Affect WLED)

There is a subtle spec ambiguity: some definitions indicate that pointerleave, pointerout, pointerenter, and pointerover should not be fired immediately (without a pointer move) when an element under the pointer is removed by a DOM API or hidden/moved by a style change. However, since the WLED tooltip only removes its own .tooltip child element (not the target slider element), this edge case doesn't apply here.


🏁 Summary

Concern Old (pointerover/out) New (pointerenter/leave)
Browser support ✅ All major ✅ All major
Child element re-firing ❌ Yes (causes "null" bug) ✅ No
Mouse hover ✅ Works ✅ Works
Touch (drag) ⚠️ Non-functional during capture ⚠️ Same (unchanged)
Pen/stylus hover ✅ Works ✅ Works
Unforeseen regressions None identified

The change is semantically correct, well-supported across all major browsers and OSes, and actually aligns with what MDN explicitly recommends for this exact use case. No unforeseen negative consequences were found.


🌐 Web query:

pointerenter pointerleave vs pointerover pointerout browser compatibility differences

💡 Result:

Behavioral differences

Event pair Bubbles? Fires when moving between child elements inside the same parent? “Entered/left” meaning
pointerover / pointerout Yes Yes (because the event target changes as you cross child boundaries) Crosses the hit-test boundary of the current target element (can be a child). [2]
pointerenter / pointerleave No No (they don’t bubble, and pointerleave waits until you leave the element and all descendants) pointerenter: into element or any descendant; pointerleave: out of element and all descendants. [2]

Also: if you use pointer capture (setPointerCapture), browsers suppress pointerover/pointerout/pointerenter/pointerleave while capture is active (because retargeting prevents normal hit-testing). [3]

Browser compatibility differences (practical)

  • In modern browsers, all four are available as part of the Pointer Events API (Chrome/Edge, Firefox, Safari, iOS Safari). Safari support starts at Safari 13 (and iOS Safari 13.2+ listed as supported). [1]
  • IE10 is a notable outlier: Pointer Events are partial there, and the partial-support note is specifically that it lacks pointerenter and pointerleave. (IE11 supports Pointer Events.) [1]
  • MDN also marks pointerenter as “widely available” (Baseline) across browsers since July 2020, reflecting that modern evergreen browser versions have long supported it. [4]

Rule of thumb: use pointerenter/pointerleave when you want “hover-like” semantics that don’t spam when moving across children; use pointerover/pointerout when you need bubbling/delegation (but expect extra events as the target changes). [2]

Sources: [1] Can I use: Pointer Events support table; [2] W3C Pointer Events spec (Level 2 working draft) event definitions/bubbling; [3] MDN Pointer Events (pointer capture note); [4] MDN pointerenter event page (Baseline statement).


🌐 Web query:

pointerenter pointerleave touch device behavior mobile browser support 2024

💡 Result:

What pointerenter / pointerleave do on touch devices

  • On devices that don’t support hover (typical phones/tablets), browsers may dispatch pointerenter as a result of pointerdown (not from “hovering”), because the finger only becomes a “pointer” when it touches the screen. (developer.mozilla.org)
  • pointerleave fires when the pointer leaves the element’s hit-test bounds; for touch this generally corresponds to the contact ending / moving away, not a hover-out. (developer.mozilla.org)

Mobile browser support status (relevant for 2024)

  • Pointer Events (and thus pointerenter/pointerleave) are supported in iOS Safari from 13.2+, with partial support in iOS Safari 13, and no support in iOS Safari 12.5 and earlier. (caniuse.com)
  • On modern Android browsers (Chromium-based), Pointer Events support is broadly available. (caniuse.com)

iOS Safari / WebKit gotchas you should still account for

  • Historical WebKit behavior: pointerenter/pointerleave (and related over/out) might not fire unless a page also registers pointerdown/pointermove/pointerup listeners (bug was marked fixed, but it’s a common legacy pitfall when testing older iOS/Safari or embedded WebViews). (bugs.webkit.org)
  • WebKit has had edge-case bugs where iOS Safari could emit unexpected pointerenter/pointerleave sequences and/or unexpected pointerType values in specific situations (e.g., focusable elements, DOM removal timing). (bugs.webkit.org)

Practical guidance (2024-era best practice)

  • For “hover-like” UI, don’t rely on pointerenter/leave alone on mobile; prefer explicit interaction events (pointerdown, pointerup, click) and design mobile states accordingly. (developer.mozilla.org)
  • If you use pointerenter/leave for styling, consider gating hover UX with CSS media queries like (any-hover: hover) rather than assuming touch can “hover” (touch typically can’t). (developer.mozilla.org)

Citations:


@DedeHai: Here's a thorough cross-browser/OS evaluation of the pointerover/pointeroutpointerenter/pointerleave change:


🔍 Core Behavioral Difference

The key distinction is bubbling:

Event pair Bubbles? Re-fires when crossing child element boundaries?
pointerover / pointerout ✅ Yes ✅ Yes — root cause of the "null" bug
pointerenter / pointerleave ❌ No ❌ No — fires once on entry/exit of the whole element+descendants

The old pair fires again whenever the pointer crosses a child element boundary (because the event target changes), while pointerenter/pointerleave wait until the pointer has truly left the element and all its descendants before firing. This is precisely why the "null" bug occurred.


✅ Desktop Browser Support

pointerenter is marked "widely available" (Baseline) across browsers since July 2020, meaning all modern evergreen browser versions have long supported it. This covers Chrome, Firefox, Safari, and Edge without any concern.

The only notable outlier is IE10, which has partial Pointer Events support and specifically lacks pointerenter and pointerleave — IE11 and all modern browsers support them. IE10 is not a relevant target for WLED.


📱 Touch / Mobile

On devices that don't support hover (typical phones/tablets), browsers may dispatch pointerenter as a result of pointerdown (not from "hovering"), because the finger only becomes a "pointer" when it touches the screen. pointerleave fires when the pointer leaves the element's hit-test bounds, which for touch generally corresponds to the contact ending or moving away.

This is the same general behavior as before with pointerover/pointerout — tooltips on touch were already non-interactive hover UIs, so no regression here.

iOS Safari support for Pointer Events (including pointerenter/pointerleave) starts at iOS Safari 13.2+, with partial support in iOS 13 and no support in 12.5 and earlier. Given that WLED targets modern devices, this is not a practical concern.


⚠️ One Minor iOS/WebKit Caveat

There was a historical WebKit bug where pointerenter/pointerleave might not fire unless a page also registers pointerdown/pointermove/pointerup listeners — though this bug was marked as fixed. If WLED's tooltip code is the only pointer event listener on those elements, very old embedded WebViews could theoretically miss the events, but this is an unlikely edge case for WLED's use case.


⚙️ Pointer Capture (Slider Dragging)

When pointer capture (setPointerCapture) is active — which browsers invoke implicitly during slider dragging — all four event types (pointerover, pointerout, pointerenter, pointerleave) are suppressed, since retargeting prevents normal hit-testing. This means slider-drag behavior is identical between old and new code.


🏁 Summary

Scenario Old (pointerover/out) New (pointerenter/leave)
Desktop Chrome/Firefox/Edge ✅ Works (but buggy) ✅ Works correctly
Desktop Safari ✅ Works (but buggy) ✅ Works correctly
Child element re-firing ❌ Yes → causes "null" bug ✅ No
Touch (mobile/tablet) ⚠️ No hover, non-functional ⚠️ Same (unchanged)
Slider drag (pointer capture) ⚠️ Suppressed by capture ⚠️ Same (unchanged)
iOS Safari 12.5- ✅ Supported ❌ Not supported
IE10 ✅ Supported ❌ Not supported

Verdict: The change is semantically correct, aligns with the W3C recommended approach for hover-like semantics, and has no unforeseen negative consequences for WLED's supported browser targets. The only platforms that lose support (IE10, iOS ≤12.5) are effectively irrelevant for WLED's use case. The old code was actually the problematic implementation.

@pReya
Copy link
Copy Markdown
Author

pReya commented May 8, 2026

I can't recreate the initial problem anymore, myself. I only cared to validate the fix (which obviously works, when the initial problem isn't even there). I don't know how I managed to create the initial error state in the first place (it just happened during regular usage). Since I can't recreate it, I don't think anything should be fixed, until we know what the initial problem was. So I'm closing this PR, but I'll keep the Issue open if that's okay. Maybe other people will come encounter this same error state.

@pReya pReya closed this May 8, 2026
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.

Tooltip on top slider shows "null"

2 participants