Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Disable cross filtering on charts with no dimensions #30176

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

kgabryje
Copy link
Member

@kgabryje kgabryje commented Sep 6, 2024

SUMMARY

Cross filtering feature is designed to apply filters based on a dimension that user clicked in the source chart.
In some chart plugins, when the chart has no dimensions (only metric series), click on a metric would emit an empty cross filter, providing confusing user experience.
This PR disables cross filtering on charts that have no dimensions set.

Additionally, some charts were marked as compatible with cross filtering feature (behaviors: [Behavior.InteractiveChart]), but were in fact not implementing the feature. In those charts I removed the behavior and added a TODO comment to implement cross filtering.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

  1. Go to "Featured Charts" dashboard.
  2. Verify that charts with dimensions (e.g. Area, Box Plot, Gauge, Pie) still emit cross filters as before
  3. Verify that charts with no dimensions (e.g. Mixed) do not emit cross filters - nothing happens on click, cross filtering option in context menu is disabled
  4. Add more charts with no dimensions to the dashboard, verify that they don't emit cross filters after clicking on a metric series.

ADDITIONAL INFORMATION

  • Has associated issue: Cross-filtering is broken for charts that don't have dimensions #30056
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@kgabryje kgabryje changed the title fix: Disable crossfiltering on charts with no dimensions fix: Disable cross filtering on charts with no dimensions Sep 6, 2024
@dosubot dosubot bot added dashboard:cross-filters Related to the Dashboard cross filters viz:charts Namespace | Anything related to viz types labels Sep 6, 2024
@kgabryje
Copy link
Member Author

kgabryje commented Sep 6, 2024

/testenv up

Copy link
Contributor

github-actions bot commented Sep 6, 2024

@kgabryje Ephemeral environment spinning up at http://18.246.229.36:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

Copy link
Member

@michael-s-molina michael-s-molina left a comment

Choose a reason for hiding this comment

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

@kgabryje @yousoph I have no problem with cutting the scope down for 4.1.0 and simply disabling cross filter for charts with no dimensions. Keep in mind though that this should not be the final solution as our x-axis supports categorical, temporal and numeric types which could be used in a cross filter. One simple example is when a user creates a bar chart and the x-axis is categorical. By the way, Drill By works for this scenario so I would expect cross filters to work too.

Screen.Recording.2024-09-06.at.09.05.04.mov

@sadpandajoe sadpandajoe added the v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch label Sep 6, 2024
@kgabryje kgabryje merged commit 3aafd29 into apache:master Sep 9, 2024
38 of 40 checks passed
Copy link
Contributor

github-actions bot commented Sep 9, 2024

Ephemeral environment shutdown and build artifacts deleted.

@kgabryje
Copy link
Member Author

kgabryje commented Sep 9, 2024

@michael-s-molina Yes I 100% agree that we should integrate cross-filtering with x-axis. This PR aims to temporarily remove confusing user experience and unblock 4.1 release.
Drill by is a bit different case - it doesn't take x-axis into consideration, it simply ignores the original chart's dimension (since it's not there)

@sadpandajoe sadpandajoe linked an issue Sep 9, 2024 that may be closed by this pull request
3 tasks
sadpandajoe pushed a commit that referenced this pull request Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dashboard:cross-filters Related to the Dashboard cross filters plugins size/M v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch viz:charts Namespace | Anything related to viz types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cross-filtering is broken for charts that don't have dimensions
3 participants