feat(dashboards): Filter status indicators #10936

suddjian · 2020-09-17T16:02:36Z

SUMMARY

This adds a new filter indicator which is intended to take the place of the "teeth" style indicators. This indicator is lower profile, and less confusing. If a filter is ineligible for a given chart, that is now displayed on the indicator.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

On the left, a chart with 2 filters applied. On the right, a chart with a filter that could not be applied.

TEST PLAN

ADDITIONAL INFORMATION

graceguo-supercat · 2020-09-18T22:01:26Z

Having both dropdown and teeth indicator at the same time? is it a little confusing and duplicated?

junlincc · 2020-09-21T02:32:11Z

yes, the teeth will be gone~

codecov-commenter · 2020-09-22T08:26:57Z

Codecov Report

Merging #10936 into master will decrease coverage by 5.30%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #10936      +/-   ##
==========================================
- Coverage   66.05%   60.75%   -5.31%     
==========================================
  Files         816      383     -433     
  Lines       38589    24195   -14394     
  Branches     3650        0    -3650     
==========================================
- Hits        25491    14700   -10791     
+ Misses      12996     9495    -3501     
+ Partials      102        0     -102

Flag	Coverage Δ
#cypress	`?`
#javascript	`?`
#python	`60.75% <100.00%> (-0.66%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/views/core.py	`74.16% <ø> (ø)`
superset/viz.py	`57.62% <100.00%> (+0.17%)`	⬆️
superset/db_engines/hive.py	`0.00% <0.00%> (-85.72%)`	⬇️
superset/db_engine_specs/hive.py	`53.90% <0.00%> (-30.08%)`	⬇️
superset/db_engine_specs/presto.py	`70.60% <0.00%> (-11.36%)`	⬇️
superset/examples/world_bank.py	`97.10% <0.00%> (-2.90%)`	⬇️
superset/examples/birth_names.py	`97.36% <0.00%> (-2.64%)`	⬇️
superset/views/database/mixins.py	`80.70% <0.00%> (-1.76%)`	⬇️
superset/models/core.py	`87.56% <0.00%> (-0.83%)`	⬇️
superset/connectors/sqla/models.py	`89.70% <0.00%> (-0.14%)`	⬇️
... and 408 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e337355...a4fa527. Read the comment docs.

CoryChaplin · 2020-09-22T19:44:41Z

A personal opinion: I really liked the colour part of the filters that allowed to view in a glimpse what filter was where.
The number and details of the filters is interesting too, but don't we loose a part of the info (and the nice colours in the filter box)?

junlincc · 2020-09-22T21:16:02Z

Thanks for the comment, Cory! As we are slowly moving towards our new design, proposed in SIP34, https://projects.invisionapp.com/share/BNVBOEJRWQ2#/screens/398392315 we try to create better consistency and cleanness across the product. During our research, we noticed that when there are too many charts and filters in a dashboard, the colorful indicators on the side become visually overwhelming, and the indicators take up lots of space in the dashboard. we went through multiple phases of research and design exploration to come up with the new solution. Going forward, our design process will be more transparent and we will present the finding community wise. @mihir Chahuan <mihir@preset.io> please chime in if you have additional comments. [image: Screen Shot 2020-09-22 at 2.07.45 PM.png] [image: Screen Shot 2020-09-22 at 2.07.55 PM.png] [image: Screen Shot 2020-09-22 at 2.08.18 PM.png] [image: Screen Shot 2020-09-22 at 2.08.25 PM.png] [image: Screen Shot 2020-09-22 at 2.08.31 PM.png]

…

On Tue, Sep 22, 2020 at 12:44 PM CoryChaplin ***@***.***> wrote: A personal opinion: I really liked the color part of the filters that allowed to view in a glimpse what filter was where. The number and details of the filters is interesting too, but don't we loose a part of the info (and the nice colours in the filter box)? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#10936 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQFR5U7VP5EELBSPJYAYCULSHD5DRANCNFSM4RQU4IOA> .

CoryChaplin · 2020-09-23T16:58:05Z

Thanks for the feedback @junlinc , I get your point. Goodbye colours :)

junlincc · 2020-09-23T17:09:56Z

Once we implement the new dashboard native filter with a filter bar as a whole, this indicator new design will probably make more sense!

…

On Wed, Sep 23, 2020 at 9:58 AM CoryChaplin ***@***.***> wrote: Thanks for the feedback @junlinc <https://github.com/JunlinC> , I get your point. Goodbye colours :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10936 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQFR5U4Q7PPCSR3ROFOYUMTSHISK3ANCNFSM4RQU4IOA> .

superset-frontend/images/icons/filter.svg

superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel.tsx

graceguo-supercat · 2020-09-24T20:45:37Z

@suddjian according to SIP-45 discussion, when one or more filters are not applicable to the chart, we will still show the chart with query results, but not applied with wrong filter_fields. We should not show "No Results".

suddjian · 2020-09-25T18:15:30Z

@suddjian according to SIP-45 discussion, when one or more filters are not applicable to the chart, we will still show the chart with query results, but not applied with wrong filter_fields. We should not show "No Results".

The chart in the screenshot is only showing "No Results" because there genuinely are no results for that query. That is not because of the filters being unapplicable.

superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel.tsx

ktmud · 2020-09-25T20:51:48Z

superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts

+type Datasource = {
+  time_grain_sqla?: [string, string][];
+  granularity?: [string, string][];
+};


can probably replace with import { DatasourceMeta } from '@superset-ui/chart-controls'

Nice, thanks

that type doesn't include granularity. It probably should, right?

Also, the type for time_grain_sqla appears to be incomplete. It's string and should be [string, string][] (or maybe the union of those?)

Hmm, @superset-ui/chart-controls@v15.0.3 (which is in the master branch) should have made time_grain_sqla [string, string][]: https://github.com/apache-superset/superset-ui/blob/0f11940d8201feea3c5f5adee82aef68a9d201f3/packages/superset-ui-chart-controls/src/types.ts#L72

Based on
https://github.com/apache/incubator-superset/blob/fb6f14c4693c5764cc2a151c27746386b190eeae/superset/connectors/sqla/models.py#L661-L664

I believe it is only [string, string][] for Datasource.

As for granularity, feel free to add it in superset-ui/chart-controls if it's not too much hassle for you.

There is also import { Datasource } from '@superset/core', which is missing both properties. We should probably consolidate these two instead of adding yet another new type.

superset-frontend/src/dashboard/components/FiltersBadge/index.tsx

graceguo-supercat · 2020-09-26T01:09:12Z

I want to raise a usability concern about the new design @junlincc
Before:

when charts are loaded in dashboard, from filter indicators I can tell each chart should be applicable to any dashboard filters or not, even I don't actually apply any filters to this dashboard.
when i focus/interact with any filter_box dropdown, I can see which charts will be affected by this filter field, even I don't actually apply the filter. This is really useful when user want to double check filter scope is correct.

After:
All these indication functionalities are lost. I would think these lost as regression. Could you bring them into new design?

BEFORE:

AFTER:

….tsx Co-authored-by: Evan Rusackas <evan@preset.io>

…set into feature/filter-p0

superset-frontend/src/dashboard/stylesheets/components/chart.less

….tsx Co-authored-by: Evan Rusackas <evan@preset.io>

superset-frontend/src/dashboard/stylesheets/components/chart.less

….tsx Co-authored-by: Evan Rusackas <evan@preset.io>

…set into feature/filter-p0

rusackas

Left some nits, mostly about Theme variables, but I'll give this the green light and let you clean that up before merging as we discussed.

mistercrunch · 2020-10-29T05:40:40Z

* Initial commit of new filters badge. * refactor applied/rejected filters code * finished filter indicators * filter badge tested * unnecessary imports * formatting and types * fixes * license * code quality tweaks * state management for showing focused filter scope * clean up filter key extraction code * remove unnecessary styles * temp css to demonstrate highlighting * fix focused filter logic * no more color badges * new toys for highlighting dash components (apache#11144) * tweak style for the filter chart when filter is focused * style: Filters p0 css2 (apache#11151) * nixing background tweak * src paths * another quick theme color * src paths, adjusting pill icon color, changing icons, showing applied/busted counts * linting stuff * fixing and tweaking tests * show filter indicator when filters are not active * chart title bar cleanup * open the right panel when popover opens * unused import * fix EditableTitle tests * margin on dashboard header * show the chart dropdown menu * fix blur filter breaking dropdowns * style tweak - no pointer events when irrelevant charts are blurred * fix box shadow on filter highlight * it's an array * attempt fixing e2e * style: filters p0 icon churn (apache#11215) * new filters icon * icon styling * bigger icons in list views * better sizing of table actions and favStars * more icon sizing... * fixing more button size jankiness * linting * Filters performance (apache#11255) * fixing time filter "ok" button * making unset filter menu collapsible * sort alphabetically * fix highlighting when removing items * try a flex layout (for browser render perf) * more specific transitioning * temp: comment out some code as a test * temp: comment out more code * temp: remove possibly expensive computations from ChartHolder * Revert "temp: comment out some code as a test" This reverts commit 309b880. * Revert "temp: comment out more code" This reverts commit 64c88b2. * Revert "temp: remove possibly expensive computations from ChartHolder" This reverts commit 37ce021. * experiment: upgrade react-select to v3 * Revert "experiment: upgrade react-select to v3" This reverts commit c3972ba. * fix the damn problem * remove code used for testing purposes * awful hack to avoid adding a class to a container * approaching infinity... and not beyond! * fix ref forwarding * add theme to tests as necessary * fix(extra-filters): add logic for identifying applied extra filters (apache#11325) * fix: use dashboard id for stable cache key (apache#11293) * fix: button translations missing (apache#11187) * button translations missing * blank space before text * feat: update time_compare description and choices (apache#11294) * feat: update time_compare description and choices * Update sections.jsx * fix(extra-filters): add logic for identifying applied extra filters * lint Co-authored-by: Jesse Yang <jesse.yang@airbnb.com> Co-authored-by: rubenSastre <ruben.sastre@decathlon.com> Co-authored-by: Erik Ritter <erik.ritter@airbnb.com> * address design feedback * slight tweak to panel logic, keep panels open that user has opened * rearrange code to be more graceful * fix: bump superset-ui/core (apache#11385) * use is_dttm instead of is_temporal * types, names * only show unset filter panel if there are unset filters * fix highlighting the filter control * fix filterbox layout * translations * fix cypress * actually add the test attribute * Update superset-frontend/src/dashboard/components/DashboardBuilder.jsx Co-authored-by: Evan Rusackas <evan@preset.io> * Update superset-frontend/src/dashboard/components/DashboardBuilder.jsx Co-authored-by: Evan Rusackas <evan@preset.io> * formatting * add link comment to hack * Update superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx Co-authored-by: Evan Rusackas <evan@preset.io> * stop importing lodash * Update superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx Co-authored-by: Evan Rusackas <evan@preset.io> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <evan@preset.io> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <evan@preset.io> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <evan@preset.io> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <evan@preset.io> * skip broken test * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <evan@preset.io> * Update superset-frontend/src/dashboard/components/FiltersBadge/Styles.tsx Co-authored-by: Evan Rusackas <evan@preset.io> * adjust colors of titles * linting * no indicators when chart is loading * support all time fields * fix lock file Co-authored-by: Natalie Ruhe <natalie@preset.io> Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Co-authored-by: Jesse Yang <jesse.yang@airbnb.com> Co-authored-by: rubenSastre <ruben.sastre@decathlon.com> Co-authored-by: Erik Ritter <erik.ritter@airbnb.com> Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com>

superset-github-bot bot added the preset-io label Sep 17, 2020

pull-request-size bot added the size/XL label Sep 17, 2020

suddjian force-pushed the feature/filter-p0 branch from d7e926e to 45354fc Compare September 22, 2020 08:20

pull-request-size bot added size/XXL and removed size/XL labels Sep 22, 2020

This comment has been minimized.

Sign in to view

suddjian marked this pull request as ready for review September 22, 2020 18:31

suddjian requested a review from graceguo-supercat September 23, 2020 18:01

rusackas reviewed Sep 24, 2020

View reviewed changes

superset-frontend/images/icons/filter.svg Outdated Show resolved Hide resolved

rusackas reviewed Sep 24, 2020

View reviewed changes

superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel.tsx Show resolved Hide resolved

ktmud reviewed Sep 25, 2020

View reviewed changes

superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel.tsx Outdated Show resolved Hide resolved

ktmud reviewed Sep 25, 2020

View reviewed changes

superset-frontend/src/dashboard/components/FiltersBadge/index.tsx Outdated Show resolved Hide resolved

superset-frontend/src/dashboard/components/FiltersBadge/index.tsx Outdated Show resolved Hide resolved

Natalie Ruhe and others added 7 commits September 28, 2020 12:05

Initial commit of new filters badge.

2666a1a

refactor applied/rejected filters code

f6e0bb9

finished filter indicators

30e0dce

filter badge tested

3cfd116

unnecessary imports

4ada45d

formatting and types

6e6e8ce

fixes

47e5bc7

suddjian and others added 6 commits October 27, 2020 16:47

Update superset-frontend/src/dashboard/components/FiltersBadge/Styles…

9c4441d

….tsx Co-authored-by: Evan Rusackas <evan@preset.io>

Update superset-frontend/src/dashboard/components/FiltersBadge/Styles…

473f83d

….tsx Co-authored-by: Evan Rusackas <evan@preset.io>

Update superset-frontend/src/dashboard/components/FiltersBadge/Styles…

d4ff8b0

….tsx Co-authored-by: Evan Rusackas <evan@preset.io>

Update superset-frontend/src/dashboard/components/FiltersBadge/Styles…

b303705

….tsx Co-authored-by: Evan Rusackas <evan@preset.io>

skip broken test

68922c0

Merge branch 'feature/filter-p0' of github.com:apache/incubator-super…

6d8386b

…set into feature/filter-p0

rusackas reviewed Oct 27, 2020

View reviewed changes

superset-frontend/src/dashboard/stylesheets/components/chart.less Show resolved Hide resolved

rusackas reviewed Oct 28, 2020

View reviewed changes

superset-frontend/src/dashboard/stylesheets/components/chart.less Show resolved Hide resolved

Update superset-frontend/src/dashboard/components/FiltersBadge/Styles…

b2c9794

….tsx Co-authored-by: Evan Rusackas <evan@preset.io>

rusackas reviewed Oct 28, 2020

View reviewed changes

superset-frontend/src/dashboard/stylesheets/components/chart.less Show resolved Hide resolved

suddjian and others added 3 commits October 27, 2020 17:01

Update superset-frontend/src/dashboard/components/FiltersBadge/Styles…

4da3791

….tsx Co-authored-by: Evan Rusackas <evan@preset.io>

adjust colors of titles

eee3f00

Merge branch 'feature/filter-p0' of github.com:apache/incubator-super…

98d3735

…set into feature/filter-p0

rusackas approved these changes Oct 28, 2020

View reviewed changes

suddjian and others added 5 commits October 27, 2020 19:51

linting

4ee56bf

no indicators when chart is loading

3972edc

support all time fields

4d8aeca

Merge branch 'master' into feature/filter-p0

79ff611

fix lock file

17e9f6d

suddjian merged commit 18658f4 into master Oct 28, 2020

graceguo-supercat mentioned this pull request Oct 29, 2020

[dashboard tab] cursor shape #11471

Closed

rusackas mentioned this pull request Oct 29, 2020

fix: Unwinds icon fallout that came along with filters PR #11474

Merged

6 tasks

rusackas mentioned this pull request Oct 29, 2020

fix: dashboard tab only shows move cursor when editing #11480

Merged

6 tasks

suddjian deleted the feature/filter-p0 branch October 31, 2020 04:30

kgabryje mentioned this pull request May 19, 2021

feat(native-filters): Highlight charts affected by focused native filter #14693

Merged

8 tasks

graceguo-supercat mentioned this pull request May 21, 2021

[native filter] highlight charts do not work #14757

Closed

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dashboards): Filter status indicators #10936

feat(dashboards): Filter status indicators #10936

suddjian commented Sep 17, 2020 •

edited

Loading

graceguo-supercat commented Sep 18, 2020

junlincc commented Sep 21, 2020

This comment has been minimized.

codecov-commenter commented Sep 22, 2020 •

edited

Loading

CoryChaplin commented Sep 22, 2020 •

edited

Loading

junlincc commented Sep 22, 2020 via email

CoryChaplin commented Sep 23, 2020

junlincc commented Sep 23, 2020 via email

graceguo-supercat commented Sep 24, 2020 •

edited

Loading

suddjian commented Sep 25, 2020

ktmud Sep 25, 2020

suddjian Sep 28, 2020

suddjian Sep 28, 2020

suddjian Sep 28, 2020

ktmud Sep 28, 2020

graceguo-supercat commented Sep 26, 2020 •

edited

Loading

rusackas left a comment

mistercrunch commented Oct 29, 2020

feat(dashboards): Filter status indicators #10936

feat(dashboards): Filter status indicators #10936

Conversation

suddjian commented Sep 17, 2020 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

graceguo-supercat commented Sep 18, 2020

junlincc commented Sep 21, 2020

This comment has been minimized.

codecov-commenter commented Sep 22, 2020 • edited Loading

Codecov Report

CoryChaplin commented Sep 22, 2020 • edited Loading

junlincc commented Sep 22, 2020 via email

CoryChaplin commented Sep 23, 2020

junlincc commented Sep 23, 2020 via email

graceguo-supercat commented Sep 24, 2020 • edited Loading

suddjian commented Sep 25, 2020

ktmud Sep 25, 2020

Choose a reason for hiding this comment

suddjian Sep 28, 2020

Choose a reason for hiding this comment

suddjian Sep 28, 2020

Choose a reason for hiding this comment

suddjian Sep 28, 2020

Choose a reason for hiding this comment

ktmud Sep 28, 2020

Choose a reason for hiding this comment

graceguo-supercat commented Sep 26, 2020 • edited Loading

rusackas left a comment

Choose a reason for hiding this comment

mistercrunch commented Oct 29, 2020

suddjian commented Sep 17, 2020 •

edited

Loading

codecov-commenter commented Sep 22, 2020 •

edited

Loading

CoryChaplin commented Sep 22, 2020 •

edited

Loading

graceguo-supercat commented Sep 24, 2020 •

edited

Loading

graceguo-supercat commented Sep 26, 2020 •

edited

Loading