Skip to content

Fixes 29634: Offset data quality tooltip from chart point - #31006

Open
shah-harshit wants to merge 5 commits into
mainfrom
fix-29634-data-quality-tooltip
Open

Fixes 29634: Offset data quality tooltip from chart point#31006
shah-harshit wants to merge 5 commits into
mainfrom
fix-29634-data-quality-tooltip

Conversation

@shah-harshit

@shah-harshit shah-harshit commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Describe your changes:

Fixes #29634

Scopes data quality tooltip activation to the visible status dot and keeps the tooltip interactive after it opens. The tooltip is anchored to the triggering dot with a 4px gap, so chart background movement cannot move it away from the pointer. Incident links use compact task IDs while retaining the legacy thread fallback.

Screen.Recording.2026-08-05.at.3.48.33.PM.mov

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

High-level design:

The graph owns controlled tooltip state populated by custom status dots. It supplies Recharts with a fixed position because ComposedChart replaces the tooltip coordinate with the live chart pointer. A short close delay bridges the 4px dot-to-tooltip gap and is cancelled while the pointer is inside the tooltip. Task-first incident metadata remains compatible with existing thread data.

Tests:

Use cases covered

  • Hovering or focusing a visible result dot opens its tooltip without expanding the activation target.
  • Moving across the chart background toward the tooltip does not move or close it.
  • Keeping the pointer inside the tooltip preserves access to the incident link.
  • Incident links show compact task IDs and retain the legacy thread fallback.

Unit tests

  • I added unit tests for the new/changed logic.
  • Files updated: TestSummaryGraph.test.tsx, TestSummaryCustomTooltip.test.tsx, and TestSummaryGraphUtils.test.ts.
  • Focused result: 33 tests passed across 3 suites.

Backend integration tests

  • Not applicable (no backend API changes).

Ingestion integration tests

  • Not applicable (no ingestion changes).

Playwright (UI) tests

  • Added TestSummaryTooltip.spec.ts as a focused browser regression.
  • The test provisions a failed test result with an incident, verifies that nearby chart positions do not activate the tooltip, moves the pointer from the status dot onto the incident link, confirms the link is actually hovered, and verifies navigation after clicking it.
  • The final Playwright run is delegated to CI.

Manual testing performed

  • Verified the dot-to-tooltip pointer path in Chrome. The tooltip remained fixed, and the incident link remained visible while hovered beyond the close delay.
  • Verified formatting, design tokens, scoped TypeScript checks, and whitespace checks locally.

UI screen recording / screenshots:

The screen recording is attached above.

Checklist:

  • I have read the CONTRIBUTING document.

  • My PR title is Fixes <issue-number>: <short explanation>

  • My PR is linked to a GitHub issue via Fixes #<issue-number> above.

  • I have commented on my code where non-obvious behavior requires context.

  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

  • For UI changes: I attached a screen recording and/or screenshots above.

  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

  • I have added a test that covers the exact scenario we are fixing.

@shah-harshit
shah-harshit requested a review from a team as a code owner August 5, 2026 07:45
@shah-harshit shah-harshit added UI UI specific issues safe to test Add this label to run secure Github workflows on PRs skip-pr-checks Bypass PR metadata validation check labels Aug 5, 2026
@shah-harshit shah-harshit self-assigned this Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 2 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 2 warning(s) across 1 changed file(s).

Count Rule
2 sonarjs/no-duplicate-string
All findings
Location Rule Message
🟡 src/utils/DataQuality/TestSummaryGraphUtils.test.ts:51:24 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 23 times.
🟡 src/utils/DataQuality/TestSummaryGraphUtils.test.ts:90:13 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 11 times.

Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 66%
66.13% (77934/117838) 50.09% (47031/93886) 51.29% (14154/27593)

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

🔴 Playwright Results — workflow failed

Validated commit eea6cf178bb950b3468d30eca99b692da42b2a68 in Playwright run 31015386433, attempt 1.

✅ 264 passed · ❌ 1 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 51m 38s

⏱️ Max setup 3m 10s · max shard execution 18m 56s · max shard-job elapsed before upload 24m 39s · reporting 3s

🌐 188.52 requests/attempt · 2.04 app boots/UI scenario · 0.00% common-shard skew

Optimization targets still in progress:

  • Application boot ratio was 2.04 per UI scenario (568 boots / 279 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
🔴 Shard chromium-01 207 1 0 0 0 0
✅ Shard import-export-01 26 0 0 0 0 0
✅ Shard ingestion-01 28 0 0 0 0 0
✅ Shard reindex-01 3 0 0 0 0 0

Genuine Failures (failed on all attempts)

Features/DataQuality/TestSummaryTooltip.spec.tsTest result tooltip stays fixed while the pointer enters its incident link (shard chromium-01)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBe�[2m(�[22m�[32mexpected�[39m�[2m) // Object.is equality�[22m  Expected: �[32mtrue�[39m Received: �[31mfalse�[39m  Call Log: - Timeout 15000ms exceeded while waiting on the predicate

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@shah-harshit
shah-harshit force-pushed the fix-29634-data-quality-tooltip branch from 2026de9 to f04c1a0 Compare August 5, 2026 08:39
@gitar-bot

gitar-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 4 resolved / 4 findings

Anchors data quality tooltips to chart status dots with a 4px offset and adds a close delay to maintain interactivity, addressing previous edge clipping, fetch failure, and clipping findings.

✅ 4 resolved
Edge Case: Incident task fetch never retries after failure/empty result

📄 openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TestSummary/TestSummaryGraph.tsx:155-169
The effect sets fetchedIncidentTasksForFqn.current = testCaseFqn before calling getTaskData, and getTaskData swallows errors internally. If the fetch fails, returns no tasks, or new incidents appear for the same test case later, the ref guard permanently prevents a re-fetch, so incident links may never populate until the component remounts. Consider only marking the fqn as fetched once tasks are successfully loaded, or re-running when testCaseResults incident IDs change and no matching task was found.

Edge Case: Left-placed tooltip may overflow the chart's left edge

📄 openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TestSummary/TestSummaryGraph.utils.ts:26-35
getTooltipAnchor switches to bottom placement only when x < TOOLTIP_EDGE_THRESHOLD (200). For points just above the threshold, the left-placed tooltip (positioned at x - TOOLTIP_GAP with translate(-100%, -50%)) needs roughly its own width plus the 14px gap of clearance; if the tooltip is wider than ~186px it can render off the left edge of the chart. Verify the threshold accounts for actual tooltip width (or clamp the position), otherwise near-left points can clip the tooltip.

Edge Case: One failed incident fetch drops all tooltip task data

📄 openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TestSummary/TestSummaryGraph.tsx:166-178
In TestSummaryGraph.tsx the query fn uses Promise.all over every incident ID. If a single getTaskById call rejects (e.g., a 404 for a deleted/inaccessible incident), the entire query rejects and tasks stays [], so all points lose their resolved task metadata and fall back to the legacy thread lookup — even points whose incident fetch succeeded. Use Promise.allSettled and keep the fulfilled results so one bad ID doesn't wipe out the others.

Quality: Focus ring may be clipped by fixed-size nested status-dot SVG

📄 openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TestSummary/TestSummaryGraph.tsx:240-254 📄 openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TestSummaryCustomTooltip/test-summary-custom-tooltip.less:15-18
renderStatusDot now wraps the status circle in a nested <svg> sized exactly STATUS_DOT_SIZE (8px) with the circle radius filling it edge-to-edge. The .test-summary-point:focus-visible stroke (var(--om-space-2)) extends beyond the circle edge, but a nested SVG clips overflow by default, so the outer half of the focus ring is likely cut off. Consider adding overflow="visible" to the nested svg (or sizing it larger than the dot) so the focus indicator renders fully.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source

@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs skip-pr-checks Bypass PR metadata validation check UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Data quality graph hover tooltip overlaps with data point

1 participant