-
Notifications
You must be signed in to change notification settings - Fork 16.6k
fix(timeseries): restore ECharts tooltip after closing drill menu #37284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
fix(timeseries): restore ECharts tooltip after closing drill menu #37284
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Code Review Agent Run #3bb82aActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
CodeAnt AI finished reviewing your PR. |
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx
Outdated
Show resolved
Hide resolved
|
The current change adds a workaround to restore tooltip visibility after drill menu interactions using a mousedown listener. For a higher-layer fix, consider modifying the drill menu logic to avoid hiding the tooltip altogether, perhaps by adjusting ECharts options or event handling in the menu component itself. |
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI Incremental review completed. |
Code Review Agent Run #871c37Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
msyavuz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a regression test for this?
superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.test.tsx
Show resolved
Hide resolved
…ext menu interactions
Of course! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review Agent Run #dcd3e1
Actionable Suggestions - 1
-
superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.test.tsx - 1
- Test assertion incorrect for restoration · Line 145-145
Review Details
-
Files reviewed - 2 · Commit Range:
2479846..debe7db- superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.test.tsx
- superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
| userEvent.click(menuItem); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByTestId('tooltip-visible')).toBeInTheDocument(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test expects tooltip visible when menu opens, but for restoration testing, it should be hidden.
Code suggestion
Check the AI-generated fix before applying
| expect(screen.getByTestId('tooltip-visible')).toBeInTheDocument(); | |
| expect(screen.queryByTestId('tooltip-visible')).not.toBeInTheDocument(); |
Code Review Run #dcd3e1
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
SUMMARY
Right-clicking a Timeseries/Area chart to open the drill menu caused ECharts tooltips to stop appearing until page refresh. Tooltips are gated by an “in context menu” state, but visibility was not restored on close.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
After
Before
tooltip-failing.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION