Skip to content

fix: maintain pagination state on long echarts legends. - #33728

Closed
rusackas wants to merge 1 commit into
masterfrom
fix-long-echarts-legends
Closed

rusackas wants to merge 1 commit into
masterfrom
fix-long-echarts-legends

Conversation

@rusackas

@rusackas rusackas commented Jun 9, 2025

Copy link
Copy Markdown
Member

SUMMARY

Updates the ECharts handler to store (and restore) legend page, after someone clicks a legend item, so it doesn't hop back to Page 1.

Fixes #32387

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@korbit-ai

korbit-ai Bot commented Jun 9, 2025

Copy link
Copy Markdown

Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment /korbit-review.

Your admin can change your review schedule in the Korbit Console

@rusackas
rusackas marked this pull request as ready for review June 9, 2025 23:32
@github-actions

github-actions Bot commented Jun 9, 2025

Copy link
Copy Markdown
Contributor

@rusackas Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

@dosubot dosubot Bot added the viz:charts:echarts Related to Echarts label Jun 9, 2025

@korbit-ai korbit-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Performance Redundant Legend Position Restorations ▹ view 🧠 Not in standard
Functionality Race condition in legend position restoration ▹ view 🧠 Incorrect
Files scanned ​
File Path Reviewed
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx ✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

Comment on lines +146 to +155
// Use requestAnimationFrame instead of setTimeout for better alignment with the browser's rendering cycle
requestAnimationFrame(() => {
const echartInstance = echartRef.current?.getEchartInstance();
if (echartInstance) {
echartInstance.dispatchAction({
type: 'legendScroll',
scrollDataIndex: currentLegendPage,
});
}
});

This comment was marked as resolved.

@github-actions

github-actions Bot commented Jun 9, 2025

Copy link
Copy Markdown
Contributor

@rusackas Ephemeral environment spinning up at http://35.88.182.12:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

@rusackas

Copy link
Copy Markdown
Member Author

Welp... that didn't QUITE work... seems the requestAnimationFrame is not working reliably. We could add a timeout, but this really seems to need an ECharts lifecyle (refreshed) hook of some sort.

@rusackas
rusackas marked this pull request as draft June 10, 2025 21:41
@amaannawab923

Copy link
Copy Markdown
Contributor

Welp... that didn't QUITE work... seems the requestAnimationFrame is not working reliably. We could add a timeout, but this really seems to need an ECharts lifecyle (refreshed) hook of some sort.

Hey @rusackas

https://github.com/apache/superset/pull/27766/files I wrote this pr for this
The problem with legend scrolling is our viz plugins do a complete re render , causing it to lose all local variables
In this pr , I created a variable outside the Viz plugin , which worked out , At that time , i wasnt really good with the pr flow so wasnt able to get this pass

But maybe we can use this pr as reference

@rusackas rusackas added the cares:preset Preset cares about this issue label Jun 13, 2025
@rusackas

Copy link
Copy Markdown
Member Author

Closing in favor of #33779

@rusackas rusackas closed this Jun 16, 2025
@dosubot dosubot Bot mentioned this pull request Jun 24, 2025
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cares:preset Preset cares about this issue plugins review:draft viz:charts:echarts Related to Echarts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Line chart legend resets to the first page when clicking a dimension on another page.

3 participants