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

[SIP-45] Proposal for improved UX for dashboard filter eligibility #9935

Closed
john-bodley opened this issue May 27, 2020 · 15 comments
Closed

[SIP-45] Proposal for improved UX for dashboard filter eligibility #9935

john-bodley opened this issue May 27, 2020 · 15 comments
Labels
sip Superset Improvement Proposal

Comments

@john-bodley
Copy link
Member

john-bodley commented May 27, 2020

[SIP] Proposal for improved UX for dashboard filter eligibility

Motivation

Currently a user can configure dashboard filters to either apply (enabled; the chart is non-immune to the filter) or not apply (disabled; the chart is immune to the filter) to specific charts. For the former there is no validation on whether the filter is eligible, i.e., can be applied to the datasource, for the specific chart and currently if the filter is deemed irrelevant it is simply ignored by the chart. The issue with this approach is there is no visual indication that the chart has ignored said filter(s) and thus the viewer could be misguided into believing a set of filters have been applied, thus making an incorrect conclusion.

This SIP proposes a couple of visual treatments to provide more clarify with regards to filter viability.

Proposed Change

The root problem is there is no validation if enabled filters are actually eligible for said charts. We propose two (somewhat related) approaches. We prefer the first approach from a simplicity and consistency standpoint as immunity is treated in a consistent manner.

1. Ensuring eligibility when configuring filter scopes [Preferred]

The first (pseudo-proactive) approach is ensuring filters can only be enabled for the charts which are eligible. Figure 1 shows how scoped filters are currently enabled by the user. A modification could be to only allow filters to be selected if they relate to said chart, i.e., if the "region" filter is deemed ineligible for the "Treemap" chart the checkbox would be disabled and thus the green filter pill will not appear next to the "Treemap" chart in the dashboard.

Screen Shot 2020-05-28 at 2 36 05 PM

Figure 1: Configuring filter scopes.

The dashboard UX is relatively intuitive as users can easily determine which filters are being applied to said chart based on the filter pills knowing that by construction all the defined filters for said chart are eligible.

Note for this option a migration would be required in order to uncheck any existing enabled filters which do not apply to the chart.

2. Ensuring eligibility at the chart

The second (retroactive) approach would be to modify the UI treatment based on the filter eligibility. There are two different states/scenarios we need to consider in order to provide the appropriate UX:

  1. A user is interacting with a dashboard chart where during a prior state the chart was deemed correct however after applying an ineligible filter the chart is deemed incorrect.
  2. A user is interacting with a dashboard chart where an existing (possibly saved) ineligible filter has been applied where the chart is deemed incorrect. Note there is no prior valid state for the chart.

The lefthand portion of Figure 2 illustrates these two states. The upper-left corner shows the treatment for (1) where the previously rendered chart is greyed out (via an opacity layer) and a message is show. The lower-left corner shows the treatment for (2) where since there was no prior valid state only the message is shown.

The upper-right portion of the figure provides context as to which filters don't apply (which contain an exclamation point), and the lower-right portion shows the message treatment being applied to a smaller chart.

Filters_safe

Figure 2: The various visual treatments for handling filters which do not apply to a chart. The lefthand portion illustrates the the two different states, the upper-right portion provides context as to which filters don't apply (which contain an exclamation point), and the lower-right portion shows the message treatment being applied to a smaller chart.

Note the issue will this approach is we treat charts which are immune to a filter and charts which have the filter enabled though not eligible differently, i.e., the former will be rendered where as the later will have the error message. The inconsistent UX is why we prefer the former approach.

New or Changed Public Interfaces

For both approaches, in order for the frontend to know if a filter can be applied a new RESTful endpoint needs to be added which would leverage the metadata of the underlying datasource. The endpoint should have the ability to handle multiple charts in bulk.

For the first approach if the bulk RESTful API request was made by the dashboard this would be a blocking request during the dashboard mount and would add a speculative 0.5s to the dashboard load time. An alternative non-blocking approach would be for each chart to perform said check as part of the data fetch and propagate the response to the frontend which would then retroactively disable the filter scopes. This is somewhat akin to option two but with different messaging, i.e., rather than the filter pill containing an exclamation point, the pill simply wouldn't exits as said filter doesn’t apply to the ineligible chart.

The one caveat of this approach is the filter_values Jinja macro, which per the documentation states:

This is useful if:

  • you want to use a filter box to filter a query where the name of filter box
    column doesn't match the one in the select statement
  • you want to have the ability for filter inside the main query for speed
    purposes

This could be problematic because it is not viable to determine whether the filter is applicable by simply inspecting the column/metric metadata of the underlying datasource. There are two approaches to remedying this problem:

  1. Extend the metadata to include the SQL for virtual datasources which needs to be parsed.
  2. Deprecate the filter_values Jinja macro. The first point can be circumvented by adding a virtual column (alias) to a SQL datasource which adheres to the filter where the expression is merely the name of the corresponding column. The second point cannot be circumvented resulting in sub-optimal queries.

Note for context < 0.1% of SQL datasources and < 0.1% of slices at Airbnb use the filter_values macro.

New dependencies

N/A.

Migration Plan and Compatibility

The first option would require a one off database migration to retroactively remove all filters which have been enabled for a chart even though said chart is ineligible.

Rejected Alternatives

N/A.

@john-bodley john-bodley added the sip Superset Improvement Proposal label May 27, 2020
@ktmud
Copy link
Member

ktmud commented May 28, 2020

Two immediate issues come to my mind:

  1. IIRC, filter indicators do not show up for non-applicable charts. If we are to adopt this design, does that mean all charts will have all filters indicators? That might turn problematic when a dashboard has many filters, especially if filters and charts are in different tabs. (Although this is solvable by redesign the indicators.)
  2. Blocking users from viewing a chart when not all filters are applicable sounds too strict. Sometimes it makes sense to include charts using different datasources in the same dashboard, but they may not share the same filter columns.

I've used filter scopes extensively in Tableau yet never felt the need for Tableau to make it clearer what filters apply to which charts in my dashboards. The added clarity is definitely nice, but overall I feel it should probably be dashboard designers' responsibility to make filter usage unambiguous and clear---which can be achieved via adding sep lines, subsections, etc.

@john-bodley
Copy link
Member Author

@ktmud

  1. IIRC, filter indicators do not show up for non-applicable charts. If we are to adopt this design, does that mean all charts will have all filters indicators? That might turn problematic when a dashboard has many filters, especially if filters and charts are in different tabs. (Although this is solvable by redesign the indicators.)

This only impacts non-immune charts, i.e., those charts where said filter is said to apply. Note as mentioned previously there is no eligibility check on whether a filter can be applied or not.

  1. Blocking users from viewing a chart when not all filters are applicable sounds too strict. Sometimes it makes sense to include charts using different datasources in the same dashboard, but they may not share the same filter columns.

I updated the "Rejected Alternatives" section to include an option which would provide an eligibility check and thus ensure that a filter can only be enabled on a chart if it is supported by the underlying datasource.

@ktmud
Copy link
Member

ktmud commented May 28, 2020

This looks great!

I wonder whether we can also add a shortcut near filter control themselves to quickly change the filter scope like in Tableau:
image

@graceguo-supercat
Copy link

Thank you very much this SIP. ❤️
May i know what are possible technical solution to implement this feature?
When a user creates a filter and added to dashboard, how do we know which charts are applicable? I assume each chart may have complicated sub queries, Jinja macro, so a query has to hit query engine to know if the filter is valid or not?

@john-bodley
Copy link
Member Author

@graceguo-supercat in the "New or Changed Public Interfaces" section there would need to be a new RESTful API endpoint which checks the datasource metadata. The request should be relatively efficient (we could add a bulk option to check multiple charts) though it would be more complex if we need to handle the filter_values Jinja macro.

@graceguo-supercat
Copy link

@ktmud Actually show a control in the filter_box to define its own scope was one of filter scope design.
The problem is right now Superset allow filter apply, and allow chart to immune.
If we offer a control for filter be global or tab, then each chart also need a control to set which filter to immune. User needs to go through each filter to set scope, and each chart to set immune. It seems not efficient for large dashboard with large number of filter fields and charts. So this design was rejected pretty early.

@ktmud
Copy link
Member

ktmud commented May 29, 2020

Tableau also allows applying filters to selected worksheets (charts):

https://help.tableau.com/current/pro/desktop/en-us/filtering_global.htm#apply-filters-to-select-worksheets-tableau-desktop-only

It doesn't conflict with having a shortcurt control to quickly change filter scopes between the two most used modes: charts with the same datasource, and charts with related datasource (those having the same filter column).

I don't think we need to add filter selectors to charts, just a quick switch to filter charts is enough. For large dashboard, users can always go back to use the all-in-one modal, but having a quick switch would make life easier for other dashboards, which are probably more common.

@graceguo-supercat
Copy link

graceguo-supercat commented May 29, 2020

Provide an option to set filter only applicable to charts from same datasource, probably is a quick win? @john-bodley

But you know Superset has virtual datasource, a dashboard generally contains a few different datasources. So we still need a more general solution and error handling like John proposed.

@john-bodley john-bodley changed the title [SIP 45] Proposal for signifying if a dashboard filter does not apply to a chart [SIP 45] Proposal for improved UX for dashboard filter eligibility Jun 1, 2020
@john-bodley
Copy link
Member Author

@ktmud I wonder if your suggestions are probably a nice to have (for improved UX) but not required in order to implement the filter eligibility.

@john-bodley john-bodley changed the title [SIP 45] Proposal for improved UX for dashboard filter eligibility [SIP-45] Proposal for improved UX for dashboard filter eligibility Jun 4, 2020
@ktmud
Copy link
Member

ktmud commented Jun 4, 2020

Yeah, it's a nice-to-have and could be implemented separately.

@ktmud
Copy link
Member

ktmud commented Aug 7, 2020

Based on previous offline discussions with @graceguo-supercat and @john-bodley, here's what needs to be done in the near term:

1. Present more accurate filter indicators for charts

Pass all in-scope filter fields and values to charts (together with the extraFilers field), the chart slice API returns whether a field is valid (exists in datasource) or not (could be another field activeFilers in the payload). The frontend then have a way to display only actual activeFilers for a chart.

2. Clarify applicable scopes for filters in filter scope editor modal

We could take either approach below:

  1. [Easier] Add a help text explaining filters can only apply to charts whose datasources have corresponding fields
  2. [More efforts but more robust] Create an endpoint that given a list of filter field names and charts returns applicable charts for each filter field. The backend will query database schema for all associated charts and check whether they contain columns that match the filter fields. The the frontend will use this information to disable any charts in the filter scope tree that do not match certain filter.

@john-bodley
Copy link
Member Author

I prefer option (2) as I think that relates to the first (and preferred) solution proposed.

@graceguo-supercat
Copy link

graceguo-supercat commented Aug 11, 2020

This is a similar work we did before (but was reverted) #7888

@junlincc
Copy link
Member

junlincc commented Dec 19, 2020

#12148
dashboard native filter and cascading filter are available for testing!! thanks in advance for feedback, comments, critics, love and support!!

@john-bodley
Copy link
Member Author

Closing this given that filters have been redesigned and many of these issues have been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sip Superset Improvement Proposal
Projects
Status: Denied / Closed / Discarded
Development

No branches or pull requests

4 participants