feat(legend): add checkbox to display or not ALL/INV button #32346
Conversation
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Fix Detected |
|---|---|---|
 |
Remove debug console.log ▹ view | ✅ |
Files scanned
| File Path | Reviewed |
|---|---|
| superset-frontend/plugins/plugin-chart-echarts/src/Histogram/types.ts | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/Histogram/controlPanel.tsx | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/Histogram/transformProps.ts | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/controls.tsx | ✅ |
| superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Feedback and Support
superset-frontend/plugins/plugin-chart-echarts/src/Histogram/controlPanel.tsx
Outdated
Show resolved
Hide resolved
sadpandajoe
changed the title
superset-frontend/plugins/plugin-chart-echarts/src/controls.tsx
Outdated
Show resolved
Hide resolved
rusackas
added
the
review:checkpoint
|
Superset uses Git pre-commit hooks courtesy of pre-commit. To install run the following: Alternatively it is possible to run pre-commit by running pre-commit manually: |
|
@rusackas Ephemeral environment spinning up at http://52.13.110.49:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup. |
@rusackas Thanks for your feedback, comments translated :P and thanks for pre-commit tips! |
sadpandajoe
removed
the
review:checkpoint
|
@rusackas HI, do you have any idea why there is error like this in jobs review (sharded-jest-tests(6)) please? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #32346 +/- ##
===========================================
+ Coverage 60.48% 80.85% +20.36%
===========================================
Files 1931 559 -1372
Lines 76236 49811 -26425
Branches 8568 4223 -4345
===========================================
- Hits 46114 40274 -5840
+ Misses 28017 9170 -18847
+ Partials 2105 367 -1738
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Closing/reopening to kick-start CI again, and see where this gets stuck. Some tests used to be failing, so if we can't clear them from the CI side here, a rebase might resolve some. Let's see! |
|
@rusackas Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments |
|
@rusackas Ephemeral environment spinning up at http://54.149.249.109:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup. |
|
Hmm... not sure why I'm not seeing the button on the ephemeral environment. Not sure whether that's due to the PR or the ephemeral build having issues, or me missing the mark 😅 |
|
@ERGO1995 can you check the ephemeral environment above and see if I'm taking crazy pills? |
No, you didn’t take any weird pills haha. Actually, I only made the change for the 'Histogram' chart type. But if it looks good for that one, I’ll start working on the others now :)! Because in the ephemeral environment, I do see the button for a histogram. |
|
@rusackas Alright, i updated chart type that used "import { legendSection, showValueControl } from '../controls';" code. |
|
Thanks fort the clarity on the histogram-only support. Supporting the other ECharts timeseries is a big win for consistency, for sure. Looks like a rebase is needed to resolve some conflicts (probably due to the giant Theming branch) but once those are cleared, I'll re-test and merge if it looks good! Thanks for continuing to help here. |
# Conflicts: # superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx
ERGO1995
force-pushed
the
display-all-inv-legend
branch
from
a7f3648 to
7914342
Compare
Rebase done :) |
|
Looks like we're getting closer, but Superset uses Git pre-commit hooks courtesy of pre-commit. To install run the following: A series of checks will now run when you make a git commit. Alternatively it is possible to run pre-commit by running pre-commit manually: |
SUMMARY
This PR introduces a new feature that allows users to toggle the visibility of the All/Invert buttons in the legend for ECharts-based visualizations.
Previously, these buttons were always displayed, but now users can enable or disable them through a new checkbox in the Chart Options panel.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
F12 > Console) for any unexpected errors related to legend rendering.✅ Expected Outcome:
ADDITIONAL INFORMATION