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

chore: Improves the native filters UI/UX - iteration 3 #14824

Merged
merged 1 commit into from
May 26, 2021

Conversation

michael-s-molina
Copy link
Member

@michael-s-molina michael-s-molina commented May 25, 2021

SUMMARY

Improves the native filters UI/UX - iteration 3.

  • Splits the controllers and organize them into Basic and Advanced sections
  • Automatically populate and refresh the select options based on the form state
  • Removes the Populate and Refresh buttons simplifying the user experience
  • Expands the Basic section by default
  • Adds the ability to dynamically render a controller using checkboxes
  • Adjusts the tests

This work is part of the Native dashboard filter project

The items below will be handled in the next iterations:

  • Handle the correct tab selection when validating
  • Add a required icon and validation to the controllers when their checkbox is checked
  • Add a scroll bar to the left panel
  • Fix left panel layout when the scroll is active
  • Adjust fonts and colors to match the design
  • Make the tabs stick to the top while scrolling
  • Remove horizontal scroll when the column select is too wide
  • The controls with affectsDataMask are disabled unless the default value picker is enabled

The iterations below are optional but recommended:

  • Split the FiltersConfigForm into smaller components to make it easier to read
  • Unify the select components to use the AntD one. Currently, we have different selects with different themes and interactions.

@villebro @rusackas @junlincc

@villebro Can you check the sort option to see if we need additional changes to it?

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

119158355-82c1d300-ba2c-11eb-99e8-c08b8aa55f4b.mov
20210525162112090.mp4

TESTING INSTRUCTIONS

1 - Enable native filters feature flag
2 - Enter in a dashboard
3 - Add a native filter
4 - Check the screen

ADDITIONAL INFORMATION

  • Has associated issue:
  • 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

@codecov
Copy link

codecov bot commented May 25, 2021

Codecov Report

Merging #14824 (e871d02) into master (dfe030b) will increase coverage by 0.01%.
The diff coverage is 79.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14824      +/-   ##
==========================================
+ Coverage   77.62%   77.63%   +0.01%     
==========================================
  Files         962      963       +1     
  Lines       49172    49216      +44     
  Branches     6184     6192       +8     
==========================================
+ Hits        38169    38210      +41     
- Misses      10801    10806       +5     
+ Partials      202      200       -2     
Flag Coverage Δ
javascript 72.48% <79.41%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
...ts/nativeFilters/FiltersConfigModal/FilterTabs.tsx 84.21% <ø> (ø)
...eFilters/FiltersConfigModal/FiltersConfigModal.tsx 94.87% <ø> (ø)
...onfigModal/FiltersConfigForm/FiltersConfigForm.tsx 72.64% <68.00%> (+1.36%) ⬆️
...nfigModal/FiltersConfigForm/CollapsibleControl.tsx 72.22% <72.22%> (ø)
...nfigModal/FiltersConfigForm/getControlItemsMap.tsx 100.00% <100.00%> (ø)
...ters/FiltersConfigModal/FiltersConfigForm/utils.ts 93.33% <0.00%> (+20.00%) ⬆️

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 dfe030b...e871d02. Read the comment docs.

@junlincc junlincc added the dashboard:native-filters Related to the native filters of the Dashboard label May 25, 2021
Copy link
Member

@junlincc junlincc left a comment

Choose a reason for hiding this comment

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

For the next iteration, let's remove the vertical scroll in the modal.
the dropdown is cutoff, but there're a few different solution in Superset... quick solution is adding vertical scroll in the select.

Screen.Recording.2021-05-25.at.2.38.46.PM.mov

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM - tested to work as expected. Some observations:

  • for an old dashboard, the default value was showing as checked despite the selection being empty. Might have been because of bad metadata (empty array), not sure. When I resaved the filter configs, it became unchecked as expected.
  • the controls with affectsDataMask are disabled unless the default value picker is enabled. Let's fix this in a follow-up.

@villebro villebro merged commit 0c0eccb into apache:master May 26, 2021
@michael-s-molina
Copy link
Member Author

@junlincc @villebro Added your observations in the next iterations section in the PR description.

@junlincc junlincc added this to In progress in Native dashboard filters via automation May 27, 2021
@junlincc junlincc moved this from In progress to Done in Native dashboard filters May 27, 2021
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels dashboard:native-filters Related to the native filters of the Dashboard size/XL 🚢 1.3.0
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants