Skip to content

docs(html): add bills-data-grid tooltip placement reproduction (issue #2289)#2520

Open
amir-ba wants to merge 7 commits into
telekom:mainfrom
amir-ba:issue-2289-tooltip-placement
Open

docs(html): add bills-data-grid tooltip placement reproduction (issue #2289)#2520
amir-ba wants to merge 7 commits into
telekom:mainfrom
amir-ba:issue-2289-tooltip-placement

Conversation

@amir-ba

@amir-ba amir-ba commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds packages/components/src/html/bills-data-grid.html — a minimum-reproduction page served by the Stencil dev-server that recreates the bills/invoices card UI from the issue screenshot using only Scale components.

What this PR adds

A self-contained HTML example showing:

  1. Correct placementscale-tooltip wrapping a single scale-icon-process-sepa-transaction element (SEPA direct-debit icon). For a single-element trigger getBoundingClientRect() returns one rect, so the tooltip anchors 10 px below the trigger — exactly the distance default. No bug here.
  2. Bug explanation callout — embedded <aside class="bug-note"> explaining that the bug manifests when the trigger is a multiline inline text span: the full combined bounding box is used instead of the per-line box under the cursor, causing a ~65 px offset.

Playwright validation

Ran against the pre-built Stencil www output at http://localhost:3335/bills-data-grid.html:

trigger:                 { top: 319, bottom: 355, height: 36, width: 36 }
tooltip:                 { top: 365, ariaHidden: false }
distanceFromTriggerBottom: 10   ← default distance prop, correct
triggerLineRects:          1    ← single element, not multiline
sepaIconDefined:           true ← scale-icon-process-sepa-transaction renders

Root cause (diagnosis, not fixed here)

See issue #2289 and my comment for the full write-up.

In packages/components/src/components/tooltip/tooltip.tsx:

// connectedCallback
this.triggerEl = children[0] as HTMLElement;   // ← first child only

// update()
computePosition(this.triggerEl, this.tooltipEl, ...)
// uses triggerEl.getBoundingClientRect() → full combined box for multiline inline text

Fix direction: replace getBoundingClientRect() with the last client rect of the trigger element (getClientRects()) so the tooltip anchors to the line actually under the cursor.

Closes #2289

…m#2289

Adds a minimum-reproduction HTML page served by the Stencil dev-server
(packages/components/src/html/) that recreates the bills/invoices card
UI from the issue screenshot using only Scale components.

Key details:
- 4 grid rows: phone (paid), mobile (SEPA direct-debit), phone (warning), billing (info)
- Row 2 shows scale-icon-process-sepa-transaction inside a scale-tooltip
  (placement="bottom", pre-opened) to demonstrate the CORRECT placement
  for a single-element icon trigger: distanceFromTriggerBottom === 10 (the
  default distance prop) and only 1 client rect.
- The doc-note callout explains that the bug (telekom#2289) manifests with
  multiline inline-text triggers where the full combined bounding box is
  used instead of the line box under the cursor, causing a ~65 px offset.

Validated with Playwright on the pre-built Stencil www output:
  trigger: { top: 319, bottom: 355, height: 36 }
  tooltip: { top: 365 }
  distanceFromTriggerBottom: 10  ← expected, no bug here
  triggerLineRects: 1             ← single element, not multiline
@amir-ba
amir-ba requested a review from maomaoZH as a code owner June 29, 2026 13:55
Copilot AI review requested due to automatic review settings June 29, 2026 13:55

Copilot AI left a comment

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.

Pull request overview

Adds a new Stencil dev-server HTML reproduction page to demonstrate the tooltip placement behavior described in issue #2289, using a bills/invoices-style card UI built from Scale components.

Changes:

  • Added bills-data-grid.html as a self-contained reproduction page rendering a bills “data grid” card UI.
  • Included a pre-opened scale-tooltip example for validating correct placement with a single-element icon trigger.
  • Added an inline “bug note” callout explaining why the tooltip offset occurs for multiline inline text triggers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +223 to +226
<span
class="status-icon status-payment"
data-testid="sepa-icon-trigger"
>
<code>getBoundingClientRect()</code> returns one box.<br />
The bug manifests when the trigger is a <strong>multiline inline text span</strong>:
the tooltip uses the full combined box instead of the line box under the cursor.
See <code>bills-data-grid-multiline-repro.html</code> (vanilla example) for the failing case.
amir-ba added 6 commits June 29, 2026 22:44
….html

Adds a second section to bills-data-grid.html with three side-by-side cases:

  ❌ BUGGY   — multiline <span> (3 lines in 220px column)
  ✅ CORRECT — single-line <span>
  ✅ CORRECT — single icon element

Playwright validation hovering the FIRST line of the multiline trigger:
  lineCount: 3
  lineBottoms: [314, 342, 370]
  firstLineBottom: 314 px
  lastLineBottom:  370 px
  tooltipTop:      380 px
  gapFromFirstLine: 66 px  ← should be 10
  gapFromLastLine:  10 px  ← tooltip anchors to LAST line, not hovered line
  verdict: ❌ BUG: tooltip anchors to last line bottom (not first)

Also adds a geometry readout table injected by script so the numbers are
visible directly on the page without needing Playwright.
… readout

The three tooltips in the bug-reproduction section used the
attribute. Because they are below the initial viewport, Stencil called
computePosition() once at load time with wrong coordinates. When the
user later hovered the trigger, showTooltip() returned early (already
opened) and never repositioned — making the 'CORRECT' icon tooltip
appear broken too.

Fix: remove  from all repro-section tooltips so every hover
triggers a fresh update() call with correct viewport coordinates.

Also fixes the on-page geometry readout to show gap-from-first-line
(not gap-from-combined-box-bottom) so the multiline bug shows as ❌
instead of falsely ✅.

Playwright results after fix (hovering line 1 in each case):
  multiline text   lineRects=3  gapFromLine1=66px  ❌ BUG
  single-line text lineRects=1  gapFromLine1=10px  ✅ CORRECT
  icon element     lineRects=1  gapFromLine1=10px  ✅ CORRECT
…kom#2289

Adds a trigger that is one line on desktop (tooltip correct, 10px gap) but wraps to two lines when the viewport narrows, at which point hovering the first line mis-anchors the tooltip below the last line (38px gap). Also fixes a data-testid collision in the live geometry readout table by using data-row-for for row identification.
Disable cross-axis clamping for tooltip shift middleware so bottom/top tooltips do not move beside edge-adjacent icon triggers. Narrow the issue 2289 reproduction page to the single-icon horizontal placement case and add an E2E regression covering centered alignment plus the default 10px gap.
Restores the dedicated single-icon tooltip reproduction section and renders the icon inline so it remains visible even when generated icon custom elements are unavailable in the standalone HTML build.
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 placement is not correct

2 participants