tasks: include withTooltips prop in dashboard-pie-chart spec#34
Merged
Conversation
…g example The hover acceptance check added in PR #28 (RSM-3217's DoD bundle) asserts `.visx-tooltip` content. `PieChartUnresponsive` defaults to `withTooltips={false}` — no mouse handlers, no tooltip portal — so without the prop the spec fails on toBeVisible before the regression injection can even run. Observed in the dashboard-pie-chart sandbox run (PR #30): Step 4 verify-ui failed initially, agent had to diagnose visx defaults and add the prop manually before the suite went green. Fixing the spec makes the task reproducible without that out-of-band recovery. Updates both the Rendering JSX example and the final `stage.tsx` body to include `withTooltips`, plus a one-line note explaining why it's required. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@copilot review |
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
There was a problem hiding this comment.
Pull request overview
Updates the Premium Analytics dashboard pie chart task spec so agents and contributors include the withTooltips prop required for the existing hover/tooltip Playwright acceptance check to run correctly.
Changes:
- Add a short rationale explaining why
withTooltipsis required for the hover acceptance check. - Update both the inline JSX snippet and the full
stage.tsxexample to passwithTooltipstoPieChartUnresponsive. - Add a changelogger entry noting this is an internal/spec update (no user-facing package change).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| projects/packages/premium-analytics/tasks/dashboard-pie-chart.md | Updates the task’s rendering examples and adds a rationale so pie-chart-tooltip.spec.ts can exercise the tooltip/hover behavior. |
| projects/packages/premium-analytics/changelog/add-pie-chart-task-withtooltips | Records the internal tooling/spec update in the package changelogger format. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes RSM-3271
Proposed changes
Update
projects/packages/premium-analytics/tasks/dashboard-pie-chart.md's Implementation section so thePieChartUnresponsiveexample includes thewithTooltipsprop. Without it the chart skips its mouse handlers and never mounts the visx tooltip portal, so the hover acceptance check (added in PR #28 / RSM-3217's DoD bundle) fails ontoBeVisible()before the regression injection can even run.Two edits, plus a one-line rationale:
withTooltipsto the inline snippet.stage.tsxbody — addswithTooltipsto the full file example so an agent following the spec verbatim produces matching code.How was this discovered
Sandbox run of
/premium-analytics-implement-taskfor the pie-chart task (PR #30): Step 4 verify-ui failed on the first attempt because.visx-tooltipnever appeared. The agent diagnosed it correctly (PieChartUnresponsive defaults towithTooltips={false}), added the prop manually, and proceeded. The spec should have it in the first place so unattended runs don't need that out-of-band recovery.Does this pull request change what data or activity we track or use?
No — task spec / docs only, no code or behavior change.
Testing instructions
This is a docs-only update to a task spec. Read through
projects/packages/premium-analytics/tasks/dashboard-pie-chart.mdand confirm both the Rendering snippet and the finalstage.tsxexample carrywithTooltips, plus the one-line note explaining it.Related
withTooltips