fix(echarts): emit no cross-filter for clicks that are not categories - #43192
Open
bikash-barnwal wants to merge 1 commit into
Open
fix(echarts): emit no cross-filter for clicks that are not categories#43192bikash-barnwal wants to merge 1 commit into
bikash-barnwal wants to merge 1 commit into
Conversation
getCrossFilterDataMask built a data mask from any clicked name, including ones with no labelMap entry: a pie's Total graphic text when showTotal is on, the aggregated "Other" slice, or a sector with an empty name. The resolved groupbyValues came out empty while values stayed non-empty, so the `values.length === 0` guard did not fire, and because `[].every(...)` is true every groupby column was emitted as `IS NULL` -- downstream charts filtered on nothing. Return undefined for a name that is empty, or absent from labelMap and not currently selected, which is what contextMenuEventHandler already does when labelMap has no entry. Deselection is unaffected, since a selected value was resolvable when it was selected. The filter branch also treats empty groupbyValues as "no filters" so a stale selection cannot rebuild the same bogus mask. Fixes apache#42340 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dosubot
Bot
added
change:frontend
Contributor
Code Review Agent Run #22dc8aActionable 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 AI Code Review powered by |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #43192 +/- ##
=======================================
Coverage 66.64% 66.65%
=======================================
Files 2873 2873
Lines 163625 163628 +3
Branches 37767 37769 +2
=======================================
+ Hits 109053 109064 +11
+ Misses 52442 52434 -8
Partials 2130 2130
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
Clicking a pie's Total text, its aggregated Other slice, or an unnamed sector emitted a cross-filter with no dimension values, so downstream charts filtered on nothing.
getCrossFilterDataMaskbuilt a mask from whatever name was clicked. For a name with nolabelMapentry:groupbyValuescame out empty whilevaluesstayed non-empty, so thevalues.length === 0guard never fired. The mask was still built, and since[].every(...)istrue, every groupby column was emitted asIS NULL.The fix returns
undefinedwhen the clicked name is empty, or absent fromlabelMapand not currently selected. That is already howcontextMenuEventHandlerbehaves whenlabelMaphas no entry, andcrossFilteris an optional field onContextMenuFilters, soundefinedis the shape callers expect —clickEventHandleralready guards with?.dataMask.Deselection is untouched: a value present in
selectedValueswas resolvable when it was selected, so it still takes the deselect path. The filter branch additionally treats emptygroupbyValuesas "no filters", so a stale selection cannot rebuild the same bogus mask.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: clicking Total applied
IS NULLon every groupby column to the dashboard.After: the click is inert; only real sectors emit filters.
TESTING INSTRUCTIONS
776 tests across 69 suites pass. Four new cases name #42340; two of them fail on unpatched source (
Total: 1234andOtherclicks, and the empty-name click). The other two — the context-menu path and clearing the last selected value — already behaved correctly and are included as regression guards so the new early return cannot break them.Manually: dashboard with cross-filtering on, a Pie chart with Show Total (and optionally Threshold for Other), plus a second chart. Click the total text and the Other slice; no filter should be applied, while clicking a real sector still filters.
ADDITIONAL INFORMATION
🤖 Generated with Claude Code