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(dashboard): fix default filter bar visibility + add docs #18741

Merged
merged 4 commits into from Mar 23, 2022

Conversation

villebro
Copy link
Member

@villebro villebro commented Feb 15, 2022

SUMMARY

This adds docs for reserved/supported URL parameters in the dashboard view. While writing docs for the Dashboard URL params, I noticed some inconsistencies in how they work:

  • The return type for getUrlParam was missing null which it defaults to when the param is missing
  • There was no way to disable native filters via URL params the same way that the dashboard metadata flag show_native_filters does. Here we recycle the show_filters URL param to mimic the behavior of show_native_filters.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

  1. Go to a dashboard with no native filters and set expand_filters=1 on the URL to forcefully expand the Filter Bar: http://35.86.100.235:8080/superset/dashboard/10/?expand_filters=1
  2. Go to a dashboard with at least 1 native filter and set expand_filters=0 on the URL to forcefully collapse the Filter Bar: http://35.86.100.235:8080/superset/dashboard/10/?expand_filters=0
  3. Go to a dashboard and set show_filters=0 to forcefully disable native filters (handy for standalone mode): http://35.86.100.235:8080/superset/dashboard/10/?show_filters=0
  4. Go to the "Video Game Sales" dashboard and set standalone=0 to verify that it looks like the default configuration: http://35.86.100.235:8080/superset/dashboard/5/?standalone=0
  5. Set standalone=1 to see that the top nav is removed: http://35.86.100.235:8080/superset/dashboard/5/?standalone=1
  6. Set standalone=2 to see that the top nav + title is removed: http://35.86.100.235:8080/superset/dashboard/5/?standalone=2
  7. Set standalone=3 to see that the top nav + title + top tabs are removed: http://35.86.100.235:8080/superset/dashboard/5/?standalone=3
  8. Set standalone=3&show_filters=0 to make sure filter bar, nav, title and top tabs are removed: http://35.86.100.235:8080/superset/dashboard/5/?standalone=3&show_filters=0

ADDITIONAL INFORMATION

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

@villebro villebro changed the title fix(dashboard): fix default filter tab visibility + add tests fix(dashboard): fix default filter tab visibility Feb 15, 2022
@villebro villebro changed the title fix(dashboard): fix default filter tab visibility fix(dashboard): fix default filter tab visibility + add docs Feb 15, 2022
@villebro villebro changed the title fix(dashboard): fix default filter tab visibility + add docs fix(dashboard): fix default filter bar visibility + add docs Feb 16, 2022
@villebro villebro force-pushed the villebro/dashboard-url-docs branch from ea15b7d to aa1e1c6 Compare March 21, 2022 14:34
@codecov
Copy link

codecov bot commented Mar 21, 2022

Codecov Report

Merging #18741 (84b1ae1) into master (f9feb1b) will decrease coverage by 0.07%.
The diff coverage is 71.42%.

❗ Current head 84b1ae1 differs from pull request most recent head 7ae3735. Consider uploading reports for the commit 7ae3735 to get more accurate results

@@            Coverage Diff             @@
##           master   #18741      +/-   ##
==========================================
- Coverage   66.64%   66.57%   -0.08%     
==========================================
  Files        1670     1667       -3     
  Lines       64590    64419     -171     
  Branches     6506     6505       -1     
==========================================
- Hits        43047    42887     -160     
+ Misses      19860    19849      -11     
  Partials     1683     1683              
Flag Coverage Δ
javascript 51.36% <71.42%> (-0.04%) ⬇️

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

Impacted Files Coverage Δ
...-frontend/src/components/UiConfigContext/index.tsx 50.00% <0.00%> (ø)
...d/components/DashboardBuilder/DashboardBuilder.tsx 72.72% <ø> (ø)
superset-frontend/src/utils/urlUtils.ts 36.73% <ø> (ø)
...src/dashboard/components/DashboardBuilder/state.ts 70.00% <80.00%> (+1.03%) ⬆️
...rd/components/nativeFilters/FilterBar/keyValue.tsx 33.33% <100.00%> (ø)
superset/utils/cache_manager.py 93.75% <0.00%> (-6.25%) ⬇️
superset/key_value/commands/create.py 89.47% <0.00%> (-4.41%) ⬇️
...perset-frontend/src/views/components/MenuRight.tsx 65.71% <0.00%> (-1.98%) ⬇️
...tend/src/views/CRUD/data/database/DatabaseList.tsx 65.47% <0.00%> (-1.92%) ⬇️
superset-frontend/src/views/components/Menu.tsx 58.97% <0.00%> (-1.03%) ⬇️
... and 29 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 35b8a72...7ae3735. Read the comment docs.

@villebro villebro force-pushed the villebro/dashboard-url-docs branch from aa1e1c6 to fef31c4 Compare March 22, 2022 07:59
@villebro villebro force-pushed the villebro/dashboard-url-docs branch from fef31c4 to 7ae3735 Compare March 22, 2022 08:11
@villebro
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

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

Copy link
Member

@kgabryje kgabryje left a comment

Choose a reason for hiding this comment

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

Tested many combinations of standalone, show filters, expand filters, couldn't break anything. LGTM!

Copy link
Member

@jinghua-qa jinghua-qa left a comment

Choose a reason for hiding this comment

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

LGTM!

@jinghua-qa jinghua-qa added the need:qa-review Requires QA review label Mar 23, 2022
@villebro
Copy link
Member Author

Thanks @kgabryje and @jinghua-qa for review/testing!

@villebro villebro merged commit b7ecb14 into apache:master Mar 23, 2022
@villebro villebro deleted the villebro/dashboard-url-docs branch March 23, 2022 06:55
@villebro villebro added lts-v1 and removed need:qa-review Requires QA review labels Mar 23, 2022
@github-actions
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

michael-hoffman-26 pushed a commit to nielsen-oss/superset that referenced this pull request Mar 23, 2022
…18741)

* fix(dashboard): fix default filter tab visibility + add tests

* fix types

* lint

* rename docs + add double bang to length
villebro added a commit that referenced this pull request Apr 3, 2022
* fix(dashboard): fix default filter tab visibility + add tests

* fix types

* lint

* rename docs + add double bang to length

(cherry picked from commit b7ecb14)
philipher29 pushed a commit to ValtechMobility/superset that referenced this pull request Jun 9, 2022
…18741)

* fix(dashboard): fix default filter tab visibility + add tests

* fix types

* lint

* rename docs + add double bang to length
@mistercrunch mistercrunch added 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 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 lts-v1 preset-io size/M 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🚢 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants