korbit-ai[bot] commented on code in PR #31495:
URL: https://github.com/apache/superset/pull/31495#discussion_r2237733740
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/EchartsSunburst.tsx:
##########
@@ -52,45 +51,47 @@ export default function EchartsSunburst(props:
SunburstTransformedProps) {
const getCrossFilterDataMask = useCallback(
(treePathInfo: TreePathInfo[]) => {
const treePath = extractTreePathInfo(treePathInfo);
- const name = treePath.join(',');
- const selected = Object.values(selectedValues);
- let values: string[];
- if (selected.includes(name)) {
- values = selected.filter(v => v !== name);
- } else {
- values = [name];
+ const joinedThreePath = treePath.join(',');
+ const value = treePath[treePath.length - 1];
+
+ const isCurrentValueSelected =
+ Object.values(selectedValues).includes(joinedThreePath);
+
+ if (!columns?.length || isCurrentValueSelected) {
+ return {
+ dataMask: {
+ extraFormData: {
+ filters: [],
+ },
+ filterState: {
+ value: null,
+ selectedValues: [],
+ },
+ },
+ isCurrentValueSelected,
+ };
}
- const labels = values.map(value => labelMap[value]);
return {
dataMask: {
extraFormData: {
- filters:
- values.length === 0 || !columns
- ? []
- : columns.slice(0, treePath.length).map((col, idx) => {
- const val = labels.map(v => v[idx]);
- if (val === null || val === undefined)
- return {
- col,
- op: 'IS NULL' as const,
- };
- return {
- col,
- op: 'IN' as const,
- val: val as (string | number | boolean)[],
- };
- }),
+ filters: [
+ {
+ col: columns[treePath.length - 1],
+ op: '==' as const,
+ val: value,
+ },
+ ],
Review Comment:
### Incomplete hierarchical filtering in Sunburst chart <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The filter is only being applied to the last level of the tree path,
ignoring parent levels. This doesn't maintain the hierarchical relationship in
the Sunburst chart's filtering.
###### Why this matters
Users might see incorrect data when filtering because parent-child
relationships in the hierarchy are not preserved. For example, if there are two
nodes with the same name at different levels or under different parents, the
current filter would match both incorrectly.
###### Suggested change ∙ *Feature Preview*
Modify the filter generation to include all levels of the hierarchy:
```typescript
filters: treePath.map((path, idx) => ({
col: columns[idx],
op: '==' as const,
val: path,
})),
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/351d6ee9-ec4b-49f4-a70f-df13ab4bdfbb/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/351d6ee9-ec4b-49f4-a70f-df13ab4bdfbb?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/351d6ee9-ec4b-49f4-a70f-df13ab4bdfbb?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/351d6ee9-ec4b-49f4-a70f-df13ab4bdfbb?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/351d6ee9-ec4b-49f4-a70f-df13ab4bdfbb)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:5b1979b6-3a8b-433e-b4a9-5cf525d87b76 -->
[](5b1979b6-3a8b-433e-b4a9-5cf525d87b76)
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/EchartsSunburst.tsx:
##########
@@ -52,45 +51,47 @@
const getCrossFilterDataMask = useCallback(
(treePathInfo: TreePathInfo[]) => {
const treePath = extractTreePathInfo(treePathInfo);
- const name = treePath.join(',');
- const selected = Object.values(selectedValues);
- let values: string[];
- if (selected.includes(name)) {
- values = selected.filter(v => v !== name);
- } else {
- values = [name];
+ const joinedThreePath = treePath.join(',');
Review Comment:
### Typo in Variable Name <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Variable name contains a typo ('Three' instead of 'Tree') which could cause
confusion.
###### Why this matters
Typos in variable names can mislead developers about the variable's purpose
and make code harder to maintain.
###### Suggested change ∙ *Feature Preview*
```typescript
const joinedTreePath = treePath.join(',');
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f0d9d452-1bc6-4b14-a95d-29b4edc88c10/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f0d9d452-1bc6-4b14-a95d-29b4edc88c10?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f0d9d452-1bc6-4b14-a95d-29b4edc88c10?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f0d9d452-1bc6-4b14-a95d-29b4edc88c10?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f0d9d452-1bc6-4b14-a95d-29b4edc88c10)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:9b95bcac-19a4-4885-8640-274ab34cda28 -->
[](9b95bcac-19a4-4885-8640-274ab34cda28)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
