You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/01/23 22:50:03 UTC
[PR] fix(chart): enable cross-filter on bar charts without dimensions [superset]
rusackas opened a new pull request, #37407:
URL: https://github.com/apache/superset/pull/37407
## Summary
Fixes #25334 - Bar charts now emit cross-filters using the X-axis category value when no dimensions are configured.
**The Problem:**
When a bar chart has no "Dimensions" field set, clicking on a bar did nothing - cross-filtering was completely disabled. Users expected clicking on a bar (e.g., "Product A") to filter other charts by that X-axis value.
**The Fix:**
When no dimensions exist but the X-axis is categorical, use the X-axis column/value for cross-filtering:
- Added `getXAxisCrossFilterDataMask()` - creates cross-filter using X-axis column and clicked value
- Added `handleXAxisChange()` - emits the X-axis-based cross-filter
- Modified click handler to check `canCrossFilterByXAxis` (categorical X-axis without dimensions)
- Updated context menu to provide X-axis cross-filter option
**Before:** Clicking a bar with no dimensions → nothing happens
**After:** Clicking a bar with no dimensions → filters by X-axis category value
## Test plan
- [x] Added unit tests for new cross-filter behavior
- [x] Existing tests pass
- [ ] Manual testing: Create a bar chart without dimensions, click on bars to verify cross-filtering works
## Technical Details
The key insight was that the X-axis value is available in `props.data[0]` during click events, but was never used for cross-filtering. The contextmenu handler already had logic to extract this value for drill-to-detail, so the fix follows the same pattern.
🤖 Generated with [Claude Code](https://claude.ai/code)
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #37407:
URL: https://github.com/apache/superset/pull/37407#discussion_r2737539101
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -252,12 +312,18 @@ export default function EchartsTimeseries({
});
});
+ // Provide cross-filter for dimensions OR categorical X-axis (issue #25334)
+ let crossFilter;
+ if (hasDimensions) {
+ crossFilter = getCrossFilterDataMask(seriesName);
+ } else if (canCrossFilterByXAxis && data) {
+ crossFilter = getXAxisCrossFilterDataMask(data[0]);
Review Comment:
Addressed in commit 343185a — added the same `data?.[0] != null` guard to the context menu handler.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -154,6 +154,43 @@ export default function EchartsTimeseries({
[groupby, labelMap, selectedValues],
);
+ // Cross-filter using X-axis value when no dimensions are set (issue #25334)
+ const getXAxisCrossFilterDataMask = useCallback(
+ (xAxisValue: string | number) => {
+ const stringValue = String(xAxisValue);
+ const selected: string[] = Object.values(selectedValues);
+ let values: string[];
+ if (selected.includes(stringValue)) {
+ values = selected.filter(v => v !== stringValue);
+ } else {
+ values = [stringValue];
+ }
+ return {
+ dataMask: {
+ extraFormData: {
+ filters:
+ values.length === 0
+ ? []
+ : [
+ {
+ col: xAxis.label,
+ op: 'IN' as const,
+ val: values,
+ },
+ ],
+ },
+ filterState: {
+ label: values.length ? values : undefined,
+ value: values.length ? values : null,
+ selectedValues: values.length ? values : null,
+ },
+ },
+ isCurrentValueSelected: selected.includes(stringValue),
+ };
+ },
+ [selectedValues, xAxis.label],
+ );
Review Comment:
Good catch\! Addressed in commit 343185a — added a `hasDimensions` flag to the `transformSeries` call in `transformProps.ts`. When `hasDimensions: false`, the series dimming logic in `transformers.ts` is skipped, preventing the incorrect visual feedback when X-axis filtering is active. This keeps the actual filtering functionality intact while disabling the opacity-based highlight that doesn't make sense for X-axis-based cross-filtering.
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3810057798
🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/21432526599) for [343185a](https://github.com/apache/superset/commit/343185a)
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3807345560
## Bot Comment Assessment
Reviewed the automated review comments on this PR. Here's a summary of relevance:
### Copilot Comment #1 — String conversion concern
**Low relevance.** The existing dimension-based cross-filter code already converts values to strings (e.g., `String(...)` is used throughout the codebase for filter values). This follows the established pattern.
### Copilot Comments #2 & #3 — Undefined check on `props.data[0]`
**Valid minor improvement.** The code checks `props.data` for truthiness but doesn't guard against an empty array, so `props.data[0]` could be `undefined`. In practice this is unlikely to cause issues (an empty-data chart wouldn't fire click/context-menu events on data points), but adding `props.data?.[0]` would be a defensive improvement.
### Copilot Comment #4 — Visual feedback mismatch with `selectedValues`
**Valid and important.** This is a real concern. When cross-filtering by X-axis value, `selectedValues` will contain X-axis values (e.g., `"Product A"`). However, `transformers.ts` line 241 checks `!filterState?.selectedValues.includes(name)` where `name` is the **series name** (e.g., `"Sales"`). Since X-axis values won't match series names, all series will appear dimmed — the visual highlight feedback won't work correctly for X-axis cross-filtering.
This would need a fix in the opacity/highlight logic in `transformers.ts` to distinguish between dimension-based filtering (matches series names) and X-axis filtering (matches data point X values).
### bito-code-review bot
**No actionable items** — generic summary, no specific concerns raised.
### netlify bot
**Standard deploy preview link** — not a review comment.
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3811492749
⚠️ **DEPRECATED WORKFLOW** ⚠️
@EnxDev This workflow is deprecated! Please use the new **Superset Showtime** system instead:
- Replace "testenv-up" label with "🎪 trigger-start"
- Better reliability and easier management
- See https://github.com/mistercrunch/superset-showtime for details
Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/21441230640). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3812846647
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - 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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3807662820
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - 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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3811542142
@EnxDev Ephemeral environment spinning up at http://35.95.15.173:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3819154406
🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/21447121741) for [c18e125](https://github.com/apache/superset/commit/c18e125)
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3792917924
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #37407:
URL: https://github.com/apache/superset/pull/37407#discussion_r2737537815
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -154,6 +154,43 @@ export default function EchartsTimeseries({
[groupby, labelMap, selectedValues],
);
+ // Cross-filter using X-axis value when no dimensions are set (issue #25334)
+ const getXAxisCrossFilterDataMask = useCallback(
+ (xAxisValue: string | number) => {
+ const stringValue = String(xAxisValue);
+ const selected: string[] = Object.values(selectedValues);
+ let values: string[];
+ if (selected.includes(stringValue)) {
+ values = selected.filter(v => v !== stringValue);
+ } else {
+ values = [stringValue];
+ }
+ return {
+ dataMask: {
+ extraFormData: {
+ filters:
+ values.length === 0
+ ? []
+ : [
+ {
+ col: xAxis.label,
+ op: 'IN' as const,
+ val: values,
+ },
+ ],
+ },
+ filterState: {
+ label: values.length ? values : undefined,
+ value: values.length ? values : null,
+ selectedValues: values.length ? values : null,
+ },
+ },
+ isCurrentValueSelected: selected.includes(stringValue),
Review Comment:
This follows the established pattern in the codebase where filter values are converted to strings. The existing dimension-based cross-filter code already uses `String()` throughout for consistency. Since Superset's filter framework handles type coercion at the database layer, keeping string values in the filter state is the expected behavior here.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -164,18 +201,41 @@ export default function EchartsTimeseries({
[emitCrossFilters, setDataMask, getCrossFilterDataMask],
);
+ // Handle cross-filter using X-axis value when no dimensions (issue #25334)
+ const handleXAxisChange = useCallback(
+ (xAxisValue: string | number) => {
+ if (!emitCrossFilters) {
+ return;
+ }
+ setDataMask(getXAxisCrossFilterDataMask(xAxisValue).dataMask);
+ },
+ [emitCrossFilters, setDataMask, getXAxisCrossFilterDataMask],
+ );
+
+ // Determine if X-axis can be used for cross-filtering (categorical axis without dimensions)
+ const canCrossFilterByXAxis =
+ !hasDimensions && xAxis.type === AxisType.Category;
+
const eventHandlers: EventHandlers = {
click: props => {
- if (!hasDimensions) {
+ // Allow cross-filter by dimensions OR by categorical X-axis (issue #25334)
+ if (!hasDimensions && !canCrossFilterByXAxis) {
return;
}
if (clickTimer.current) {
clearTimeout(clickTimer.current);
}
// Ensure that double-click events do not trigger single click event. So we put it in the timer.
clickTimer.current = setTimeout(() => {
- const { seriesName: name } = props;
- handleChange(name);
+ if (hasDimensions) {
+ // Cross-filter by dimension (original behavior)
+ const { seriesName: name } = props;
+ handleChange(name);
+ } else if (canCrossFilterByXAxis && props.data) {
+ // Cross-filter by X-axis value when no dimensions (issue #25334)
+ const xAxisValue = props.data[0];
+ handleXAxisChange(xAxisValue);
Review Comment:
Addressed in commit 343185a — added guard `props.data?.[0] != null` to ensure we only proceed when valid data exists.
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #37407:
URL: https://github.com/apache/superset/pull/37407#discussion_r2737540455
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -419,6 +419,7 @@ export default function transformProps(
timeCompare: array,
timeShiftColor,
theme,
+ hasDimensions: groupBy.length > 0,
Review Comment:
Done! Added optional chaining in commit 7b808a4. Note: I kept using `groupBy` (the ensured array from `ensureIsArray(groupby)`) rather than the raw `groupby` prop, since `groupBy` is guaranteed to be an array. The optional chaining provides extra defensive safety.
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3811365481
🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/21440448848) for [343185a](https://github.com/apache/superset/commit/343185a)
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3792937352
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 09c44cf33c58fcf2d8e8c50f6dce92e1fcfcf205 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6973fb1c93bcd80007302007 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-37407--superset-docs-preview.netlify.app](https://deploy-preview-37407--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-37407--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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3811497293
🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/21441230585) for [343185a](https://github.com/apache/superset/commit/343185a)
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #37407:
URL: https://github.com/apache/superset/pull/37407#discussion_r2728777259
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -252,12 +312,18 @@ export default function EchartsTimeseries({
});
});
+ // Provide cross-filter for dimensions OR categorical X-axis (issue #25334)
+ let crossFilter;
+ if (hasDimensions) {
+ crossFilter = getCrossFilterDataMask(seriesName);
+ } else if (canCrossFilterByXAxis && data) {
+ crossFilter = getXAxisCrossFilterDataMask(data[0]);
Review Comment:
There's no validation that the X-axis value exists before using it. If data is an empty array or data[0] is undefined, the code will convert undefined to the string "undefined" and attempt to filter on it, which will likely cause incorrect behavior.
Consider adding a check to ensure the X-axis value is not undefined before proceeding with the cross-filter operation.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -154,6 +154,43 @@ export default function EchartsTimeseries({
[groupby, labelMap, selectedValues],
);
+ // Cross-filter using X-axis value when no dimensions are set (issue #25334)
+ const getXAxisCrossFilterDataMask = useCallback(
+ (xAxisValue: string | number) => {
+ const stringValue = String(xAxisValue);
+ const selected: string[] = Object.values(selectedValues);
+ let values: string[];
+ if (selected.includes(stringValue)) {
+ values = selected.filter(v => v !== stringValue);
+ } else {
+ values = [stringValue];
+ }
+ return {
+ dataMask: {
+ extraFormData: {
+ filters:
+ values.length === 0
+ ? []
+ : [
+ {
+ col: xAxis.label,
+ op: 'IN' as const,
+ val: values,
+ },
+ ],
+ },
+ filterState: {
+ label: values.length ? values : undefined,
+ value: values.length ? values : null,
+ selectedValues: values.length ? values : null,
+ },
+ },
+ isCurrentValueSelected: selected.includes(stringValue),
+ };
+ },
+ [selectedValues, xAxis.label],
+ );
Review Comment:
When X-axis cross-filtering is active, the selectedValues will contain X-axis values (e.g., "Product A") rather than series names (e.g., "Sales"). This causes a visual feedback issue in transformers.ts (around line 241) where the code checks `!filterState?.selectedValues.includes(name)` to determine if a series should be dimmed.
Since series names (metric names like "Sales") won't match X-axis values (like "Product A"), all series will be incorrectly marked as filtered and rendered with semi-transparent opacity. This doesn't break the actual filtering functionality (which happens via extraFormData), but provides incorrect visual feedback to users - all bars/lines will appear dimmed when X-axis filtering is active.
Consider either:
1. Disabling the opacity-based visual feedback when using X-axis filtering (by not setting selectedValues in filterState for X-axis filters)
2. Implementing a different visual feedback mechanism for X-axis filtering
3. Adding a flag to filterState to distinguish between dimension-based and X-axis-based filtering
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -154,6 +154,43 @@ export default function EchartsTimeseries({
[groupby, labelMap, selectedValues],
);
+ // Cross-filter using X-axis value when no dimensions are set (issue #25334)
+ const getXAxisCrossFilterDataMask = useCallback(
+ (xAxisValue: string | number) => {
+ const stringValue = String(xAxisValue);
+ const selected: string[] = Object.values(selectedValues);
+ let values: string[];
+ if (selected.includes(stringValue)) {
+ values = selected.filter(v => v !== stringValue);
+ } else {
+ values = [stringValue];
+ }
+ return {
+ dataMask: {
+ extraFormData: {
+ filters:
+ values.length === 0
+ ? []
+ : [
+ {
+ col: xAxis.label,
+ op: 'IN' as const,
+ val: values,
+ },
+ ],
+ },
+ filterState: {
+ label: values.length ? values : undefined,
+ value: values.length ? values : null,
+ selectedValues: values.length ? values : null,
+ },
+ },
+ isCurrentValueSelected: selected.includes(stringValue),
Review Comment:
Converting the X-axis value to a string may cause filtering issues when the X-axis column contains numeric values. The filter value type should match the column's data type. For example, if the category column contains years as numbers (2020, 2021, etc.), converting them to strings ('2020', '2021') may not match the column type in the database, causing the filter to fail.
Consider preserving the original type of xAxisValue instead of converting it to a string. The selectedValues comparison on line 163 could use String() for comparison purposes, but the actual filter value should retain its original type.
```suggestion
const selected = Object.values(
selectedValues,
) as (string | number)[];
const selectedStrings = selected.map(v => String(v));
let filterValues: (string | number)[];
let stateValues: string[];
if (selectedStrings.includes(stringValue)) {
// Deselect current value
filterValues = selected.filter(v => String(v) !== stringValue);
stateValues = selectedStrings.filter(v => v !== stringValue);
} else {
// Select current value
filterValues = [...selected, xAxisValue];
stateValues = [...selectedStrings, stringValue];
}
return {
dataMask: {
extraFormData: {
filters:
filterValues.length === 0
? []
: [
{
col: xAxis.label,
op: 'IN' as const,
val: filterValues,
},
],
},
filterState: {
label: stateValues.length ? stateValues : undefined,
value: stateValues.length ? stateValues : null,
selectedValues: stateValues.length ? stateValues : null,
},
},
isCurrentValueSelected: selectedStrings.includes(stringValue),
```
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -164,18 +201,41 @@ export default function EchartsTimeseries({
[emitCrossFilters, setDataMask, getCrossFilterDataMask],
);
+ // Handle cross-filter using X-axis value when no dimensions (issue #25334)
+ const handleXAxisChange = useCallback(
+ (xAxisValue: string | number) => {
+ if (!emitCrossFilters) {
+ return;
+ }
+ setDataMask(getXAxisCrossFilterDataMask(xAxisValue).dataMask);
+ },
+ [emitCrossFilters, setDataMask, getXAxisCrossFilterDataMask],
+ );
+
+ // Determine if X-axis can be used for cross-filtering (categorical axis without dimensions)
+ const canCrossFilterByXAxis =
+ !hasDimensions && xAxis.type === AxisType.Category;
+
const eventHandlers: EventHandlers = {
click: props => {
- if (!hasDimensions) {
+ // Allow cross-filter by dimensions OR by categorical X-axis (issue #25334)
+ if (!hasDimensions && !canCrossFilterByXAxis) {
return;
}
if (clickTimer.current) {
clearTimeout(clickTimer.current);
}
// Ensure that double-click events do not trigger single click event. So we put it in the timer.
clickTimer.current = setTimeout(() => {
- const { seriesName: name } = props;
- handleChange(name);
+ if (hasDimensions) {
+ // Cross-filter by dimension (original behavior)
+ const { seriesName: name } = props;
+ handleChange(name);
+ } else if (canCrossFilterByXAxis && props.data) {
+ // Cross-filter by X-axis value when no dimensions (issue #25334)
+ const xAxisValue = props.data[0];
+ handleXAxisChange(xAxisValue);
Review Comment:
There's no validation that the X-axis value exists before using it. If props.data is an empty array or data[0] is undefined, the code will convert undefined to the string "undefined" and attempt to filter on it, which will likely cause incorrect behavior.
Consider adding a check to ensure the X-axis value is not undefined before proceeding with the cross-filter operation.
```suggestion
} else if (
canCrossFilterByXAxis &&
Array.isArray(props.data) &&
props.data.length > 0
) {
// Cross-filter by X-axis value when no dimensions (issue #25334)
const xAxisValue = props.data[0];
if (xAxisValue !== undefined && xAxisValue !== null) {
handleXAxisChange(xAxisValue);
}
```
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37407:
URL: https://github.com/apache/superset/pull/37407#issuecomment-3812448089
🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/21447121741) for [c18e125](https://github.com/apache/superset/commit/c18e125)
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #37407:
URL: https://github.com/apache/superset/pull/37407#discussion_r2736683106
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -419,6 +419,7 @@ export default function transformProps(
timeCompare: array,
timeShiftColor,
theme,
+ hasDimensions: groupBy.length > 0,
Review Comment:
```suggestion
hasDimensions: (groupby?.length ?? 0) > 0,
```
--
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] fix(chart): enable cross-filter on bar charts without dimensions [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #37407:
URL: https://github.com/apache/superset/pull/37407
--
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