more logical zero value tooltip position by bothness · Pull Request #562 · svelteplot/svelteplot · GitHub
Skip to content

more logical zero value tooltip position - #562

Open
bothness wants to merge 1 commit into
svelteplot:mainfrom
bothness:html-tooltip-zero-position
Open

more logical zero value tooltip position#562
bothness wants to merge 1 commit into
svelteplot:mainfrom
bothness:html-tooltip-zero-position

Conversation

@bothness

Copy link
Copy Markdown
Contributor

This PR fixes an issue with the HTMLTooltip position when the X or Y datum value is exactly zero.

Related issue #561

@netlify

netlify Bot commented May 12, 2026

Copy link
Copy Markdown

@ljodea ljodea left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Investigation summary

Validated #561 against main. The root cause is the truthy check on tooltipX / tooltipY in HTMLTooltip.svelte (lines 102–103): when a datum has x === 0 or y === 0, the ternary falls through to 0px instead of the projected position. Datum selection is unaffected; only CSS positioning is wrong.

The ?? 0 fix in this PR is correct and minimal. Hidden-tooltip behavior is unchanged.

Approving the fix. One gap before merge: please add regression tests in packages/svelteplot/tests/htmltooltip.test.svelte.ts (existing harness covers datum selection but not position styles).

Requested tests

  1. y = 0 — data [{ x: 50, y: 0 }], plot width=200 height=100, domains [0, 100]. Hover near the projected point and assert tooltip.style.top is not '0px' and is close to 100 (bottom of plot).

  2. x = 0 with centered domain — data [{ x: 0, y: 50 }], x.domain = [-50, 50]. Hover at center and assert tooltip.style.left is not '0px' and is close to 100.

These fail on main and pass with this fix — good TDD coverage for the falsy-zero case.

@ljodea ljodea assigned ljodea and unassigned ljodea Jun 22, 2026
@ljodea

ljodea commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

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.

2 participants