You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "SBIN2010 (via GitHub)" <gi...@apache.org> on 2026/10/10 16:03:33 UTC

[PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

SBIN2010 opened a new pull request, #45172:
URL: https://github.com/apache/superset/pull/45172

   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   This continues the work in #32346 and follows the request in discussion #27012 to hide the All/Inv legend buttons. Those buttons are shown on ECharts legends by default, and there was no chart option to turn them off. #32346 added a checkbox for this, but it was closed before merge.
   Chart options now include **Show All/Invert buttons in legend**, stored as `show_selector_legend`. It is visible only while the legend itself is shown, and it defaults to on so existing charts keep the buttons.
   The control lives in the shared legend section, so it applies to the ECharts charts that use that section: timeseries (line, smooth, bar, area, scatter, step), mixed timeseries, pie, radar, funnel, graph, bubble, butterfly, candlestick, and Gantt. Histogram does not use that section, so the same checkbox is added next to its existing legend control.
   Turning the checkbox off hides the buttons on the native ECharts legend and on the custom HTML legend used by plain top/bottom timeseries. The layout no longer reserves space for those buttons. Charts saved before this field existed still show the buttons.
   Funnel no longer drops a legend row by a fixed index. The row is removed by control name (`legendOrientation`), so inserting this checkbox does not shift which control disappears. That replaces the old `splice(2, 1)`, which removed the legend type control.
   New strings are added to `superset/translations/messages.pot`.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   <img width="2190" height="1248" alt="Снимок экрана от 2026-10-10 18-55-33" src="https://github.com/user-attachments/assets/3b1c31dd-a835-4581-b54d-6f4c035b6c21" />
   <img width="2190" height="1248" alt="Снимок экрана от 2026-10-10 18-55-42" src="https://github.com/user-attachments/assets/1541d902-f884-4811-b7bd-b8f2f45c8f7c" />
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   1. Open Explore and create or edit an ECharts chart that shows a legend (for example a time-series line, a pie, and a histogram).
   2. In Chart options, under Legend, confirm **Show All/Invert buttons in legend** appears only when **Show legend** is checked.
   3. With the checkbox on, confirm the All/Invert buttons are visible.
   4. Turn the checkbox off and run the query. The buttons disappear and the rest of the legend stays. On a plain top or bottom timeseries legend, the HTML All/Inverse buttons disappear as well and the plot does not keep an empty gap for them.
   5. Turn the legend off and confirm the All/Invert checkbox is hidden.
   6. Open a saved chart that has no `show_selector_legend` in its form data and confirm the buttons are still shown.
   7. Open a funnel chart and confirm the legend controls are the ones the funnel panel is supposed to show.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ x] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [x ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #45172:
URL: https://github.com/apache/superset/pull/45172#issuecomment-6099680598

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/45172?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 65.29%. Comparing base ([`b9e8188`](https://app.codecov.io/gh/apache/superset/commit/b9e8188ad606f61317403c3be5ef9450dbe0e381?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`5b975e1`](https://app.codecov.io/gh/apache/superset/commit/5b975e1df103e281b17d5a9de77169ce092a290a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   > :exclamation:  There is a different number of reports uploaded between BASE (b9e8188) and HEAD (5b975e1). Click for more details.
   > 
   > <details><summary>HEAD has 4 uploads less than BASE</summary>
   >
   >| Flag | BASE (b9e8188) | HEAD (5b975e1) |
   >|------|------|------|
   >|python|7|3|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #45172       +/-   ##
   ===========================================
   - Coverage   82.85%   65.29%   -17.56%     
   ===========================================
     Files        3017     3017               
     Lines      192923   192873       -50     
     Branches    44976    44953       -23     
   ===========================================
   - Hits       159841   125938    -33903     
   - Misses      30042    64139    +34097     
   + Partials     3040     2796      -244     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/45172/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/45172/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `35.30% <ø> (ø)` | |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/45172/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/45172/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/45172/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.13% <ø> (ø)` | |
   | [python](https://app.codecov.io/gh/apache/superset/pull/45172/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.58% <ø> (-32.92%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/45172/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.46% <ø> (ø)` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/45172/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/45172?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4256176400


##########
superset-frontend/plugins/plugin-chart-echarts/src/Funnel/controlPanel.tsx:
##########
@@ -36,8 +36,17 @@ import { legendSection } from '../controls';
 const { labelType, numberFormat, showLabels, defaultTooltipLabel } =
   DEFAULT_FORM_DATA;
 
-const funnelLegendSection = [...legendSection];
-funnelLegendSection.splice(2, 1);
+// Funnel keeps legend type and drops orientation; the row is matched by name
+// so inserting another legend control does not remove the wrong one.

Review Comment:
   The comment still says the opposite of what the filter does.
   
   ```suggestion
   // Funnel keeps legend orientation and drops type; the row is matched by name
   // so inserting another legend control does not remove the wrong one.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4249755294


##########
superset-frontend/plugins/plugin-chart-echarts/src/Funnel/controlPanel.tsx:
##########
@@ -36,8 +36,17 @@ import { legendSection } from '../controls';
 const { labelType, numberFormat, showLabels, defaultTooltipLabel } =
   DEFAULT_FORM_DATA;
 
-const funnelLegendSection = [...legendSection];
-funnelLegendSection.splice(2, 1);
+// Funnel keeps legend type and drops orientation; the row is matched by name
+// so inserting another legend control does not remove the wrong one.
+const funnelLegendSection = legendSection.filter(row => {
+  const [control] = row;
+  return (
+    !control ||
+    typeof control !== 'object' ||
+    !('name' in control) ||
+    control.name !== 'legendOrientation'

Review Comment:
   Good point. Corrected in b409b04



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238553220


##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -1466,6 +1466,36 @@ test('moves a visible horizontal Plain legend into a custom HTML legend and rest
   expect('contentHeight' in transformed).toBe(false);
 });
 
+test('hides legend selector buttons when showSelectorLegend is false', () => {
+  const plain = transformProps(
+    createTestChartProps({
+      formData: {
+        ...formData,
+        legendOrientation: LegendOrientation.Top,
+        legendType: LegendType.Plain,
+        showLegend: true,
+        showSelectorLegend: false,
+      },
+    }),
+  );
+  expect(getCustomLegend(plain)?.showSelectors).toBe(false);
+
+  const scroll = transformProps(
+    createTestChartProps({
+      formData: {
+        ...formData,
+        legendOrientation: LegendOrientation.Top,
+        legendType: LegendType.Scroll,
+        showLegend: true,
+        showSelectorLegend: false,
+      },
+    }),
+  );
+  expect((scroll.echartOptions.legend as { selector?: unknown }).selector).toBe(
+    false,
+  );

Review Comment:
   ✅ **CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved** as of `69e2370`.
   
   The test checks that `selectorLabel.show` is false for the scroll legend, matching the label visibility setting returned by `getLegendProps`.
   
   <sub>If that's not right, unresolve this thread and CodeAnt will leave it open.</sub>
   
   <!-- codeant-auto-resolve-reply -->



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238611860


##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -919,8 +921,14 @@ export function getLegendProps(
         }
       : {}),
     selected: legendState ?? {},
-    selector: ['all', 'inverse'],
+    // `selectorLabel.show` only hides the button text. `selector: false` removes
+    // the All/Inv buttons themselves.
+    selector: [
+      { type: "all", title: t("All") },
+      { type: "inverse", title: t("Inv") },
+    ],

Review Comment:
   The buttons are hidden by ECharts itself—specifically via the show parameter in selectorLabel.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238285411


##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -1823,6 +1826,7 @@ export default function transformProps(
         legendOrientation,
         nativeLegendVisible,
         theme,
+        showSelectorLegend,

Review Comment:
   **Suggestion:** `getLegendProps` only uses this value for `selectorLabel.show` and keeps `selector` enabled, so native legends still show the All/Inv controls when unchecked.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Sometimes` · 🏷️ `Api mismatch`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=880a1547ac1e434f95655bd5d91735c6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=880a1547ac1e434f95655bd5d91735c6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
   **Line:** 1829:1829
   **Comment:**
   	*Api Mismatch: `getLegendProps` only uses this value for `selectorLabel.show` and keeps `selector` enabled, so native legends still show the All/Inv controls when unchecked.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=5db70462d8c63d39c6ea740867d369d30d1485220f5579c7a63c22526c1bb962&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=5db70462d8c63d39c6ea740867d369d30d1485220f5579c7a63c22526c1bb962&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238287553


##########
superset-frontend/plugins/plugin-chart-echarts/src/Histogram/transformProps.ts:
##########
@@ -185,6 +186,7 @@ export default function transformProps(
         LegendOrientation.Top,
         showLegend,
         theme,
+        showSelectorLegend,

Review Comment:
   **Suggestion:** When this is false, `getLegendProps` hides only selector labels and still includes the selector controls, so the All/Inv buttons remain.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Sometimes` · 🏷️ `Api mismatch`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5528f6954c8941ecbe143f90feb2a423&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=5528f6954c8941ecbe143f90feb2a423&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/plugins/plugin-chart-echarts/src/Histogram/transformProps.ts
   **Line:** 189:189
   **Comment:**
   	*Api Mismatch: When this is false, `getLegendProps` hides only selector labels and still includes the selector controls, so the All/Inv buttons remain.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=ffc7c8f04e0bcbd0a5707e3c921951100a5e9802f0e8d2a1a1363444271cd3d4&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=ffc7c8f04e0bcbd0a5707e3c921951100a5e9802f0e8d2a1a1363444271cd3d4&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238504777


##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -1466,6 +1466,36 @@ test('moves a visible horizontal Plain legend into a custom HTML legend and rest
   expect('contentHeight' in transformed).toBe(false);
 });
 
+test('hides legend selector buttons when showSelectorLegend is false', () => {
+  const plain = transformProps(
+    createTestChartProps({
+      formData: {
+        ...formData,
+        legendOrientation: LegendOrientation.Top,
+        legendType: LegendType.Plain,
+        showLegend: true,
+        showSelectorLegend: false,
+      },
+    }),
+  );
+  expect(getCustomLegend(plain)?.showSelectors).toBe(false);
+
+  const scroll = transformProps(
+    createTestChartProps({
+      formData: {
+        ...formData,
+        legendOrientation: LegendOrientation.Top,
+        legendType: LegendType.Scroll,
+        showLegend: true,
+        showSelectorLegend: false,
+      },
+    }),
+  );
+  expect((scroll.echartOptions.legend as { selector?: unknown }).selector).toBe(
+    false,
+  );

Review Comment:
   ✅ **CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved** as of `3f76c45`.
   
   The test checks that `selectorLabel.show` is false for the scroll legend, matching the label visibility setting returned by `getLegendProps`.
   
   <sub>If that's not right, unresolve this thread and CodeAnt will leave it open.</sub>
   
   <!-- codeant-auto-resolve-reply -->



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238615594


##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -1823,6 +1826,7 @@ export default function transformProps(
         legendOrientation,
         nativeLegendVisible,
         theme,
+        showSelectorLegend,

Review Comment:
   The buttons are hidden by ECharts itself—specifically via the show parameter in selectorLabel.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 merged PR #45172:
URL: https://github.com/apache/superset/pull/45172


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4329624555


##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -919,7 +921,13 @@ export function getLegendProps(
         }
       : {}),
     selected: legendState ?? {},
-    selector: ['all', 'inverse'],
+    // `selector: false` omits the All/Inv group from legend layout.
+    selector: showSelectorLegend
+      ? [
+          { type: 'all', title: t('All') },
+          { type: 'inverse', title: t('Inv') },
+        ]
+      : false,

Review Comment:
   Passing an explicit `title` skips ECharts' own locale. With the `'all'` / `'inverse'` shorthand, `LegendModel` reads the labels from the locale `Echart.tsx` registers, so French users got "Tout / Inverse" and Chinese users got "全选 / 反选". `Inv` is a new msgid with no translations, so every non-English locale will show the English "Inv" after this.
   
   Keeping the shorthand hides the buttons just the same:
   
   ```suggestion
       selector: showSelectorLegend ? ['all', 'inverse'] : false,
   ```
   
   The `t` import, the `Inv` entry in `messages.pot`, and the object-form expectations in `series.test.ts` and the Gantt test can then go back to how they were.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238611860


##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -919,8 +921,14 @@ export function getLegendProps(
         }
       : {}),
     selected: legendState ?? {},
-    selector: ['all', 'inverse'],
+    // `selectorLabel.show` only hides the button text. `selector: false` removes
+    // the All/Inv buttons themselves.
+    selector: [
+      { type: "all", title: t("All") },
+      { type: "inverse", title: t("Inv") },
+    ],

Review Comment:
   fix in 97d86ca



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238557159


##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -724,9 +725,9 @@ export function extractSeries(
     // calculations (getBaselineSeriesForStream) run, so BigInt and Number
     // values can be combined without throwing (see #36401).
     const normalized: DataRecord = {};
-    Object.keys(datum).forEach(key => {
+    Object.keys(datum).forEach((key) => {
       const value = datum[key];
-      normalized[key] = typeof value === 'bigint' ? Number(value) : value;
+      normalized[key] = typeof value === "bigint" ? Number(value) : value;
     });

Review Comment:
   fix in 3f76c45



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238563534


##########
superset-frontend/plugins/plugin-chart-echarts/src/Histogram/transformProps.ts:
##########
@@ -185,6 +186,7 @@ export default function transformProps(
         LegendOrientation.Top,
         showLegend,
         theme,
+        showSelectorLegend,

Review Comment:
   The buttons are hidden by ECharts itself—specifically via the `show` parameter in `selectorLabel`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #45172:
URL: https://github.com/apache/superset/pull/45172#issuecomment-6127388204

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | b409b0420cf510b70fb739d8c903c009591dd423 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6accfc744fdba80d367a9683 |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-45172--superset-docs-preview.netlify.app](https://deploy-preview-45172--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQ1MTcyLS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.BSQ2LZ-ucvs5SNDqu8XIoLKz31n6RFsqcQy_8DPFTi4)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#q1YqKMrPLShRslJS0lFKKkrMS84AstNSE0v0MzJTUoNTc1KTS_KLnPPzSoryc4BqUlILcvIrPVOAqswSk5PTks1NTNJSkhItDFKMzcwTLc0sjJVqAQ) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-45172--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4299026594


##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -919,8 +921,12 @@ export function getLegendProps(
         }
       : {}),
     selected: legendState ?? {},
-    selector: ['all', 'inverse'],
+    selector: [
+      { type: 'all', title: t('All') },
+      { type: 'inverse', title: t('Inv') },
+    ],
     selectorLabel: {
+      show: showSelectorLegend,

Review Comment:
   After looking into this a bit, I’ve concluded that the best way to hide it is by using `selector: false`. When `selector: false` is set, ECharts does not create a group of buttons and does not include it in the layout; as a result, neither the border nor the 10px `selectorButtonGap` remains.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238286197


##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -1466,6 +1466,36 @@ test('moves a visible horizontal Plain legend into a custom HTML legend and rest
   expect('contentHeight' in transformed).toBe(false);
 });
 
+test('hides legend selector buttons when showSelectorLegend is false', () => {
+  const plain = transformProps(
+    createTestChartProps({
+      formData: {
+        ...formData,
+        legendOrientation: LegendOrientation.Top,
+        legendType: LegendType.Plain,
+        showLegend: true,
+        showSelectorLegend: false,
+      },
+    }),
+  );
+  expect(getCustomLegend(plain)?.showSelectors).toBe(false);
+
+  const scroll = transformProps(
+    createTestChartProps({
+      formData: {
+        ...formData,
+        legendOrientation: LegendOrientation.Top,
+        legendType: LegendType.Scroll,
+        showLegend: true,
+        showSelectorLegend: false,
+      },
+    }),
+  );
+  expect((scroll.echartOptions.legend as { selector?: unknown }).selector).toBe(
+    false,
+  );

Review Comment:
   **Suggestion:** With `showSelectorLegend: false`, `getLegendProps` still returns the All/Invert selector array, so this assertion fails and the new test suite is red.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Sometimes` · 🏷️ `Api mismatch`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1ab863644dce4a758ba883e0bc7b0445&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1ab863644dce4a758ba883e0bc7b0445&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
   **Line:** 1494:1496
   **Comment:**
   	*Api Mismatch: With `showSelectorLegend: false`, `getLegendProps` still returns the All/Invert selector array, so this assertion fails and the new test suite is red.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=ecabf2675dda829e3a8f2f7598fbbee4de1be672c06f78bb2e3d8f35c5c62720&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=ecabf2675dda829e3a8f2f7598fbbee4de1be672c06f78bb2e3d8f35c5c62720&reaction=dislike'>👎</a>



##########
superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts:
##########
@@ -1257,6 +1277,18 @@ describe('getLegendProps', () => {
       ...expectedThemeProps,
     });
   });
+
+  test('hides All and Inv buttons when showSelectorLegend is false', () => {
+    expect(
+      getLegendProps(
+        LegendType.Plain,
+        LegendOrientation.Top,
+        true,
+        theme,
+        false,
+      ).selector,
+    ).toBe(false);

Review Comment:
   **Suggestion:** With `showSelectorLegend` false, `getLegendProps` still returns the selector array, so this assertion fails whenever the test runs.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Often` · 🏷️ `Api mismatch`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=56ad9a651f2646a09f5d8ddc23f4438b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=56ad9a651f2646a09f5d8ddc23f4438b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
   **Line:** 1282:1290
   **Comment:**
   	*Api Mismatch: With `showSelectorLegend` false, `getLegendProps` still returns the selector array, so this assertion fails whenever the test runs.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=975986d4871b9b3de1ea599d3944e737ec8eb1ff092c604e2f378698a9b284ca&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=975986d4871b9b3de1ea599d3944e737ec8eb1ff092c604e2f378698a9b284ca&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238295234


##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -919,8 +921,14 @@ export function getLegendProps(
         }
       : {}),
     selected: legendState ?? {},
-    selector: ['all', 'inverse'],
+    // `selectorLabel.show` only hides the button text. `selector: false` removes
+    // the All/Inv buttons themselves.
+    selector: [
+      { type: "all", title: t("All") },
+      { type: "inverse", title: t("Inv") },
+    ],

Review Comment:
   **Suggestion:** `selector` is set regardless of `showSelectorLegend`, so disabling the option still renders blank All/Inv controls.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Often` · 🏷️ `Incomplete implementation`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ff295b040ffe469caf7d89a67829e72e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ff295b040ffe469caf7d89a67829e72e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
   **Line:** 926:929
   **Comment:**
   	*Incomplete Implementation: `selector` is set regardless of `showSelectorLegend`, so disabling the option still renders blank All/Inv controls.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=6ca99651838d5dee47a1c9a5d9df212dbd899245df176fb7e3c1978fe6c05576&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=6ca99651838d5dee47a1c9a5d9df212dbd899245df176fb7e3c1978fe6c05576&reaction=dislike'>👎</a>



##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -724,9 +725,9 @@ export function extractSeries(
     // calculations (getBaselineSeriesForStream) run, so BigInt and Number
     // values can be combined without throwing (see #36401).
     const normalized: DataRecord = {};
-    Object.keys(datum).forEach(key => {
+    Object.keys(datum).forEach((key) => {
       const value = datum[key];
-      normalized[key] = typeof value === 'bigint' ? Number(value) : value;
+      normalized[key] = typeof value === "bigint" ? Number(value) : value;
     });

Review Comment:
   **Suggestion:** `extractSeries` also converts x-axis `BigInt` values; distinct categorical or numeric values above `Number.MAX_SAFE_INTEGER` can become the same coordinate and merge chart points.
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Rarely` · 🏷️ `Logic error`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9af6611f98fc4aebb95a638f3751342d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9af6611f98fc4aebb95a638f3751342d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
   **Line:** 728:731
   **Comment:**
   	*Logic Error: `extractSeries` also converts x-axis `BigInt` values; distinct categorical or numeric values above `Number.MAX_SAFE_INTEGER` can become the same coordinate and merge chart points.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=32ad2cd347b8664aaca20a50a7f0d75c68ef5f2c913bd74835756f0cb43b000d&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45172&comment_hash=32ad2cd347b8664aaca20a50a7f0d75c68ef5f2c913bd74835756f0cb43b000d&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #45172:
URL: https://github.com/apache/superset/pull/45172#issuecomment-6099571332

   ## CodeAnt PR Risk: Medium Risk
   
   - The PR needs attention before merging.
   - Native ECharts legends always receive the All/Invert selector array, even when the option is disabled, so the buttons remain visible and the new assertions fail.
   
   **Assessed commit:** `5b975e1df103`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4238555971


##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -1466,6 +1466,36 @@ test('moves a visible horizontal Plain legend into a custom HTML legend and rest
   expect('contentHeight' in transformed).toBe(false);
 });
 
+test('hides legend selector buttons when showSelectorLegend is false', () => {
+  const plain = transformProps(
+    createTestChartProps({
+      formData: {
+        ...formData,
+        legendOrientation: LegendOrientation.Top,
+        legendType: LegendType.Plain,
+        showLegend: true,
+        showSelectorLegend: false,
+      },
+    }),
+  );
+  expect(getCustomLegend(plain)?.showSelectors).toBe(false);
+
+  const scroll = transformProps(
+    createTestChartProps({
+      formData: {
+        ...formData,
+        legendOrientation: LegendOrientation.Top,
+        legendType: LegendType.Scroll,
+        showLegend: true,
+        showSelectorLegend: false,
+      },
+    }),
+  );
+  expect((scroll.echartOptions.legend as { selector?: unknown }).selector).toBe(
+    false,
+  );

Review Comment:
   fix in 3f76c45



##########
superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts:
##########
@@ -1257,6 +1277,18 @@ describe('getLegendProps', () => {
       ...expectedThemeProps,
     });
   });
+
+  test('hides All and Inv buttons when showSelectorLegend is false', () => {
+    expect(
+      getLegendProps(
+        LegendType.Plain,
+        LegendOrientation.Top,
+        true,
+        theme,
+        false,
+      ).selector,
+    ).toBe(false);

Review Comment:
   fix in 3f76c45



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] feat(plugin-chart-echarts): let users hide legend All/Invert buttons [superset]

Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 commented on code in PR #45172:
URL: https://github.com/apache/superset/pull/45172#discussion_r4330031132


##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -919,7 +921,13 @@ export function getLegendProps(
         }
       : {}),
     selected: legendState ?? {},
-    selector: ['all', 'inverse'],
+    // `selector: false` omits the All/Inv group from legend layout.
+    selector: showSelectorLegend
+      ? [
+          { type: 'all', title: t('All') },
+          { type: 'inverse', title: t('Inv') },
+        ]
+      : false,

Review Comment:
   Good catch; ECharts does indeed perform translation based on the locale.
   Fixed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org