Skip to content

fix(echarts): stop the zoom controls overlapping the legend selector - #44367

Merged
rusackas merged 4 commits into
masterfrom
ultraset-fix-37286
Sep 29, 2026
Merged

rusackas merged 4 commits into
masterfrom
ultraset-fix-37286

Conversation

@sadpandajoe

@sadpandajoe sadpandajoe commented Sep 17, 2026 •

Copy link
Copy Markdown
Member

SUMMARY

On Timeseries-family charts with Zoomable enabled, the zoom toolbox
(zoom area / restore zoom) and the legend both render in the chart's
top-right corner, and they overlap — see #37286.

getLegendProps reserves TIMESERIES_CONSTANTS.legendTopRightOffset px of
clearance at the right edge for a top-oriented legend when zoomable is set.
That was 55px. Measuring what ECharts 6 actually renders, the toolbox's two
dataZoom icons occupy from 67px to 20px in from the right edge (the
component rect reaches 82px). So the legend's right edge — which includes its
own All/Inv selector buttons, since ScrollableLegendView.layoutInner folds
the selector group into the legend's mainRect — landed roughly 12px inside
the icon band.

This raises legendTopRightOffset to 90, clearing the icons with margin.

The offset also feeds getHorizontalLegendAvailableWidth, so plain legends
get a correspondingly more accurate wrap estimate for zoomable charts.

Note: the earlier #38657 addressed the bottom dataZoom slider offset, which is
a different control and not what this issue reports.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE
Снимок экрана от 2026-09-28 20-46-22
AFTER
Снимок экрана от 2026-09-28 20-46-45

TESTING INSTRUCTIONS

  1. Create a Time-series Line Chart with enough series that the legend
    paginates (default legend type "Scroll", position "Top").
  2. In Customize, enable Zoomable.
  3. Confirm the zoom area / restore zoom icons in the top-right no longer
    sit under the legend's All/Inv buttons, and both remain clickable.
  4. Toggle Zoomable off and confirm the legend still extends to the right edge.

Unit tests:

cd superset-frontend
npm run test -- plugins/plugin-chart-echarts/test/utils/series.test.ts

ADDITIONAL INFORMATION

  • Has associated issue: Fixes Zoom Data control doesn't fit into the panel #37286
  • 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

sadpandajoe and others added 3 commits September 17, 2026 00:56
The legend's All/Inv selector buttons render at the legend's own
right edge, anchored via TIMESERIES_CONSTANTS.legendTopRightOffset
when a chart is zoomable. That offset (55px) left only ~12px of
clearance from the zoomable toolbox's dataZoom icons (which need
~43px, right-anchored at 5px), so on charts with enough series to
paginate the legend, the selector/pager controls visually collide
with the toolbox restore/zoom icons in the same top-right corner.

Widen the reserved gap so there is real safety margin between the
two independently-positioned control clusters.

Fixes #37286
The measured footprint of the zoomable toolbox is ~67px in from the
chart's right edge (echarts 6 default icon sizing), not the ~43px cited
in the previous comment. Restate the comment and the regression test
against the real measurement so the invariant is checkable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@bito-code-review

bito-code-review Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Agent Run #3ed7be

Actionable Suggestions - 0
Additional Suggestions - 1
  • superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts - 1
    • Regression guard lacks margin · Line 1070-1071
      The guard asserts `right > 67`, which is exactly the measured toolbox footprint with no safety margin. The production constant is 90, so a regression to e.g. 70 (only ~3px clearance) would still pass and silently reintroduce the Zoom Data control doesn't fit into the panel #37286 overlap. The previous test reserved a 30px margin (43+30=73). Consider asserting `right > TOOLBOX_ICONS_RIGHT_FOOTPRINT + SAFETY_MARGIN`.
Review Details
  • Files reviewed - 2 · Commit Range: 7816371..4c37812
    • superset-frontend/plugins/plugin-chart-echarts/src/constants.ts
    • superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers an incremental AI Review.

  • /review full - 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

AI Code Review powered by Bito Logo

@codecov

codecov Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.39%. Comparing base (4c77b0b) to head (fed531d).
⚠️ Report is 200 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #44367      +/-   ##
==========================================
- Coverage   80.40%   80.39%   -0.01%     
==========================================
  Files        2932     2932              
  Lines      174525   174525              
  Branches    40548    40548              
==========================================
- Hits       140319   140315       -4     
- Misses      31537    31541       +4     
  Partials     2669     2669              
Flag Coverage Δ
javascript 75.84% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…on guard

Bito review on #44367 noted the guard asserted right > 67 (the exact
measured toolbox footprint), so a regression down to e.g. 70px would
still pass despite reintroducing the #37286 overlap.
@bito-code-review

bito-code-review Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Agent Run #3b5da3

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 4c37812..fed531d
    • superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • 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 an incremental AI Review.

  • /review full - 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

AI Code Review powered by Bito Logo

@SBIN2010

Copy link
Copy Markdown
Contributor

@sadpandajoe
Could you add before-and-after screenshots?

@rusackas

Copy link
Copy Markdown
Member

Heya @sadpandajoe, checking back in on this one. Code LGTM, just need those before/after screenshots @SBIN2010 asked for before this can merge. Mind adding them?

@rusackas rusackas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just re-posting this as an actual review so it clears my requested-review status (last time was a plain comment, which doesn't). Code still LGTM -- still just need those before/after screenshots @SBIN2010 asked for before this can merge. @sadpandajoe mind adding them?

@SBIN2010

Copy link
Copy Markdown
Contributor

Just re-posting this as an actual review so it clears my requested-review status (last time was a plain comment, which doesn't). Code still LGTM -- still just need those before/after screenshots @SBIN2010 asked for before this can merge. @sadpandajoe mind adding them?

@sadpandajoe @rusackas
I added screenshots.

@rusackas
rusackas merged commit d3f5726 into master Sep 29, 2026
85 checks passed
@rusackas
rusackas deleted the ultraset-fix-37286 branch September 29, 2026 03:51
villebro pushed a commit that referenced this pull request Sep 30, 2026
…44367)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit d3f5726)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Zoom Data control doesn't fit into the panel

3 participants