[dashboard] allow user re-order top-level tabs #7390

graceguo-supercat · 2019-04-26T18:35:28Z

SUMMARY

Allow dashboard builder to drag-n-drop to re-order top-level tabs

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:
Not allowed.

After:

TEST PLAN

Open dashboard with root-level tabs
Edit dashboard
Drag-n-drop root-level tabs to re-order tabs. Save dashboard changes. Reload page. Dashboard should show the re-ordered root-level tabs.
Drag-n-drop root-level tabs to nested row-level tab. Dashboard should show warning message.
Drag-n-drop row-level tabs to top-level tabs. Save dashboard changes. Reload page. Dashboard should show the new root-level tab.

ADDITIONAL INFORMATION

REVIEWERS

@williaster @michellethomas @mistercrunch

codecov-io · 2019-04-26T18:48:34Z

Codecov Report

Merging #7390 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #7390      +/-   ##
==========================================
+ Coverage   64.89%   64.91%   +0.01%     
==========================================
  Files         428      428              
  Lines       20979    20988       +9     
  Branches     2337     2344       +7     
==========================================
+ Hits        13615    13625      +10     
+ Misses       7240     7239       -1     
  Partials      124      124

Impacted Files	Coverage Δ
...ts/src/dashboard/components/gridComponents/Tab.jsx	`68.18% <ø> (ø)`	⬆️
...sets/src/dashboard/components/DashboardBuilder.jsx	`81.81% <100%> (+0.33%)`	⬆️
...et/assets/src/dashboard/actions/dashboardLayout.js	`97.72% <100%> (+0.22%)`	⬆️
superset/assets/src/messageToasts/actions/index.js	`79.16% <0%> (+4.16%)`	⬆️

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 4fe152d...ae5f450. Read the comment docs.

williaster

LGTM overall! 🎉 Had one important comment about state in one of the actions

williaster · 2019-04-26T22:19:45Z

superset/assets/src/dashboard/actions/dashboardLayout.js

+      // we only allow
+      // - top-level tab => top-level tab: rearrange top-level tab order
+      // - nested tab => top-level tab: allow row tab become top-level tab
+      // we do not allow top-level tab become nested tab


I would add one more line that says why we don't allow top => nested since it may not be obvious, which is that it could result in two levels of nested tabs (top > nested becomes nested > nested) which is invalid.

williaster · 2019-04-26T22:20:08Z

superset/assets/src/dashboard/actions/dashboardLayout.js

+      destination.id !== rootChildId
+    ) {
+      return dispatch(
+        addInfoToast(`Can not move top level tab into nested tabs`),


Should this be a t() call?

Also, should this be a warning toast since it's an invalid action?

added t function. I saw another message, There is not enough space for this component is also info not warning. should i make both of them warning message?

Up to you, I think consistency is good so either both info or both danger sounds good to me 👍

i updated both message to danger. Will merge after CI passed.

williaster · 2019-04-26T22:22:57Z

superset/assets/src/dashboard/actions/dashboardLayout.js

    // if we moved a child from a Tab or Row parent and it was the only child, delete the parent.
    if (!isNewComponent) {
-      const { present: layout } = undoableLayout;


I think this should be an up-to-date version of state (i.e., after the possible dispatches above). If you need it above then I would call getState() again down here so it's not stale (getState always returns the most up-to-date version of state).

very important. so i reverted this change, and in the earlier place just get a value from a needed field.

williaster

LGTM! this will be a great feature for users! 💯 🏆

* filter out all nan series (#7313) * improve not rich tooltip (#7345) * Create issue_label_bot.yaml (#7341) * fix: do not save colors without a color scheme (#7347) * [wtforms] Strip leading/trailing whitespace (#7084) * [schema] Updating the datasources schema (#5451) * limit tables/views returned if schema is not provided (#7358) * limit tables/views returned if schema is not provided * fix typo * improve code performance * handle the case when table name or view name does not present a schema * Add type anno (#7342) * Updated local dev instructions to include missing step * First pass at type annotations * [schema] Updating the base column schema (#5452) * Update 937d04c16b64_update_datasources.py (#7361) * Feature flag for client cache (#7348) * Feature flag for client cache * Fix integration test * Revert "Fix integration test" This reverts commit 58434ab. * Feature flag for client cache * Fix integration tests * Add feature flag to config.py * Add another feature check * Fix more integration tests * Fix raw HTML in SliceAdder (#7338) * remove backendSync.json (#7331) * [bubbles] issue when using duplicated metrics (#7087) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 (#7359) * SUPERSET-8: Update text in docs copyright footer (#7360) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 * SUPERSET-8: Extra text in docs copyright footer * [schema] Adding commits and removing unnecessary foreign-key definitions (#7371) * Store last selected dashboard in sessionStorage (#7181) * Store last selected dashboard in sessionStorage * Fix tests * [schema] Updating the base metric schema (#5453) * Fix NoneType bug & fill the test recipients with original recipients if empty (#7365) * Added living goods as among the users of Superset (#7407) * Added living goods as among the users of Superset Living Goods is a non profit organisation with operation in africa and the middle east. We work in community health use data heavily on day to day. Superset is our platform of choice for dashboards. * Update README.md * [dashboard] allow user re-order top-level tabs (#7390) * [SQL Lab] Increase timeout threshold for offline check (#7411) * Bump FAB to 2.0.0 (#7323) * Bump FAB to 2.0.0 * [tests] whitelist SecurityApi login and refresh endpoints * [style] Fix, C812 missing trailing commas * [security] Remove SUPERSET_UPDATE_PERMS flag Registering sources needs to be performed after the views are initialized on UPDATE_PERMS=False configuration * [docs] New, FAB_UPDATE_PERMS and flask fab cli * [docs] Fix, db upgrade needs to come first, create-admin needs a db * [cli] New, superset init bootstraps all permissions for FAB and Superset * [style] Fix, flakes * [annotations] Improves UX on annotation validation, start_dttm, end_dttm (#7326) * Setting renderTrigger on label_colors (#7410) * Refactor out controlUtils.js module + unit tests (#7350) * [WiP]refactor out a controlUtils.js file * unit tests * add missing license * Addressing comments * feature: see Presto row and array data types (#7413) * Merge lastest from master into lyft-release-sp8 (#7405) * filter out all nan series (#7313) * improve not rich tooltip (#7345) * Create issue_label_bot.yaml (#7341) * fix: do not save colors without a color scheme (#7347) * [wtforms] Strip leading/trailing whitespace (#7084) * [schema] Updating the datasources schema (#5451) * limit tables/views returned if schema is not provided (#7358) * limit tables/views returned if schema is not provided * fix typo * improve code performance * handle the case when table name or view name does not present a schema * Add type anno (#7342) * Updated local dev instructions to include missing step * First pass at type annotations * [schema] Updating the base column schema (#5452) * Update 937d04c16b64_update_datasources.py (#7361) * Feature flag for client cache (#7348) * Feature flag for client cache * Fix integration test * Revert "Fix integration test" This reverts commit 58434ab. * Feature flag for client cache * Fix integration tests * Add feature flag to config.py * Add another feature check * Fix more integration tests * Fix raw HTML in SliceAdder (#7338) * remove backendSync.json (#7331) * [bubbles] issue when using duplicated metrics (#7087) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 (#7359) * SUPERSET-8: Update text in docs copyright footer (#7360) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 * SUPERSET-8: Extra text in docs copyright footer * [schema] Adding commits and removing unnecessary foreign-key definitions (#7371) * Store last selected dashboard in sessionStorage (#7181) * Store last selected dashboard in sessionStorage * Fix tests * [schema] Updating the base metric schema (#5453) * Fix NoneType bug & fill the test recipients with original recipients if empty (#7365) * feat: see Presto row and array data types (#7391) * feat: see Presto row and array data types * fix: address PR comments * fix: lint and build issues * fix: add types * add stronger type hints where possible * fix: lint issues and add select_star func in Hive * add missing pkg init * fix: build issues * fix: pylint issues * fix: use logging instead of print * Removed --console-log and superset runserver (#7421) * Fixes dashboard export button missing download and #7353 (#7427) * Added additional German translations to string file (#6604) * Added additional German translations to string file Updates to German translation files as per directions * Removed messages.json * [fix] Fixing SQL parsing issue (#7374) * add chinese translate (#7402) * Quick fix to address deadlock issue (#7434) * feat: view presto row objects in data grid (#7445) * Merge lastest from master into lyft-release-sp8 (#7405) * filter out all nan series (#7313) * improve not rich tooltip (#7345) * Create issue_label_bot.yaml (#7341) * fix: do not save colors without a color scheme (#7347) * [wtforms] Strip leading/trailing whitespace (#7084) * [schema] Updating the datasources schema (#5451) * limit tables/views returned if schema is not provided (#7358) * limit tables/views returned if schema is not provided * fix typo * improve code performance * handle the case when table name or view name does not present a schema * Add type anno (#7342) * Updated local dev instructions to include missing step * First pass at type annotations * [schema] Updating the base column schema (#5452) * Update 937d04c16b64_update_datasources.py (#7361) * Feature flag for client cache (#7348) * Feature flag for client cache * Fix integration test * Revert "Fix integration test" This reverts commit 58434ab. * Feature flag for client cache * Fix integration tests * Add feature flag to config.py * Add another feature check * Fix more integration tests * Fix raw HTML in SliceAdder (#7338) * remove backendSync.json (#7331) * [bubbles] issue when using duplicated metrics (#7087) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 (#7359) * SUPERSET-8: Update text in docs copyright footer (#7360) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 * SUPERSET-8: Extra text in docs copyright footer * [schema] Adding commits and removing unnecessary foreign-key definitions (#7371) * Store last selected dashboard in sessionStorage (#7181) * Store last selected dashboard in sessionStorage * Fix tests * [schema] Updating the base metric schema (#5453) * Fix NoneType bug & fill the test recipients with original recipients if empty (#7365) * feat: see Presto row and array data types (#7391) * feat: see Presto row and array data types * fix: address PR comments * fix: lint and build issues * fix: add types * Incorporate feedback from initial PR (prematurely merged to lyft-release-sp8) (#7415) * add stronger type hints where possible * fix: lint issues and add select_star func in Hive * add missing pkg init * fix: build issues * fix: pylint issues * fix: use logging instead of print * feat: view presto row objects in data grid * fix: address feedback * fix: spacing * feat: Scheduling queries from SQL Lab (#7416) (#7446) * Merge lastest from master into lyft-release-sp8 (#7405) * filter out all nan series (#7313) * improve not rich tooltip (#7345) * Create issue_label_bot.yaml (#7341) * fix: do not save colors without a color scheme (#7347) * [wtforms] Strip leading/trailing whitespace (#7084) * [schema] Updating the datasources schema (#5451) * limit tables/views returned if schema is not provided (#7358) * limit tables/views returned if schema is not provided * fix typo * improve code performance * handle the case when table name or view name does not present a schema * Add type anno (#7342) * Updated local dev instructions to include missing step * First pass at type annotations * [schema] Updating the base column schema (#5452) * Update 937d04c16b64_update_datasources.py (#7361) * Feature flag for client cache (#7348) * Feature flag for client cache * Fix integration test * Revert "Fix integration test" This reverts commit 58434ab. * Feature flag for client cache * Fix integration tests * Add feature flag to config.py * Add another feature check * Fix more integration tests * Fix raw HTML in SliceAdder (#7338) * remove backendSync.json (#7331) * [bubbles] issue when using duplicated metrics (#7087) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 (#7359) * SUPERSET-8: Update text in docs copyright footer (#7360) * SUPERSET-7: Docker compose config version breaks on Ubuntu 16.04 * SUPERSET-8: Extra text in docs copyright footer * [schema] Adding commits and removing unnecessary foreign-key definitions (#7371) * Store last selected dashboard in sessionStorage (#7181) * Store last selected dashboard in sessionStorage * Fix tests * [schema] Updating the base metric schema (#5453) * Fix NoneType bug & fill the test recipients with original recipients if empty (#7365) * feat: see Presto row and array data types (#7391) * feat: see Presto row and array data types * fix: address PR comments * fix: lint and build issues * fix: add types * Incorporate feedback from initial PR (prematurely merged to lyft-release-sp8) (#7415) * add stronger type hints where possible * fix: lint issues and add select_star func in Hive * add missing pkg init * fix: build issues * fix: pylint issues * fix: use logging instead of print * feat: view presto row objects in data grid * fix: address feedback * fix: spacing * Workaround for no results returned (#7442) * feat: view presto row objects in data grid (#7436) * feat: view presto row objects in data grid * fix: address feedback * fix: spacing * feat: Scheduling queries from SQL Lab (#7416) * Lightweight pipelines POC * Add docs * Minor fixes * Remove Lyft URL * Use enum * Minor fix * Fix unit tests * Mark props as required * feat: Add `validate_sql_json` endpoint for checking that a given sql query is valid for the chosen database (#7422) (#7462) merge from lyft-release-sp8 to master * Adds missing metric sum__SP_RUR_TOTL (#7452) * Late import for optional lib pyhive (#7471) * Late import for optional lib pyhive * fix * fix: calendar heatmap examples (#7375) Fixing a set of examples that trip on ValueError vs TypeError * bugfix: Improve support for special characters in schema and table names (#7297) * Bugfix to SQL Lab to support tables and schemas with characters that require quoting * Remove debugging prints * Add uri encoding to secondary tables call * Quote schema names for presto * Quote selected_schema on Snowflake, MySQL and Hive * Remove redundant parens * Add python unit tests * Add js unit test * Fix flake8 linting error * [dashboard] After update filter, trigger new queries when charts are visible (#7233) * trigger query when chart is visible * add integration test * fix: alter sql columns to long text #7463 (#7476) Merge lyft-release-sp8@7bfe7bc to master * Refactor ConsoleLog (#7428) * Revised Chinese translation (#7464) * add chinese translate * edit chinese translation * druid connector: avoid using 'dimensions' for scan queries (#7377) After the following PyDruid change (contained in version 0.5.2) the Superset Histogram charts rendered with Druid data are broken: druid-io/pydruid@0a59a70 Bump the pydruid requirements accordingly in setup.py Issue: #7368

(cherry picked from commit 9e703f3)

(cherry picked from commit 103080a)

pull-request-size bot added the size/M label Apr 26, 2019

graceguo-supercat requested a review from williaster April 26, 2019 20:32

williaster reviewed Apr 26, 2019

View reviewed changes

graceguo-supercat force-pushed the gg-ReorderTab branch from 037510b to eda6479 Compare April 28, 2019 23:28

graceguo-supercat requested a review from williaster April 29, 2019 20:11

williaster approved these changes Apr 29, 2019

View reviewed changes

[dashboard] allow user re-order top-level tabs

ae5f450

graceguo-supercat force-pushed the gg-ReorderTab branch from eda6479 to ae5f450 Compare April 29, 2019 22:51

graceguo-supercat merged commit 9e703f3 into apache:master Apr 30, 2019

graceguo-supercat deleted the gg-ReorderTab branch May 3, 2019 18:20

smola mentioned this pull request May 20, 2019

Allow changing order of tabs in dashboards src-d/sourced-ui#60

Closed

michellethomas pushed a commit that referenced this pull request Jun 8, 2019

[dashboard] allow user re-order top-level tabs (#7390)

4a970cf

(cherry picked from commit 9e703f3)

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

cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024

[dashboard] allow user re-order top-level tabs (apache#7390)

0238e93

(cherry picked from commit 103080a)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dashboard] allow user re-order top-level tabs #7390

[dashboard] allow user re-order top-level tabs #7390

graceguo-supercat commented Apr 26, 2019 •

edited

Loading

codecov-io commented Apr 26, 2019 •

edited

Loading

williaster left a comment

williaster Apr 26, 2019

graceguo-supercat Apr 28, 2019

williaster Apr 26, 2019

williaster Apr 26, 2019

graceguo-supercat Apr 28, 2019 •

edited

Loading

williaster Apr 29, 2019

graceguo-supercat Apr 29, 2019

williaster Apr 26, 2019 •

edited

Loading

graceguo-supercat Apr 28, 2019

williaster left a comment

[dashboard] allow user re-order top-level tabs #7390

[dashboard] allow user re-order top-level tabs #7390

Conversation

graceguo-supercat commented Apr 26, 2019 • edited Loading

CATEGORY

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

REVIEWERS

codecov-io commented Apr 26, 2019 • edited Loading

Codecov Report

williaster left a comment

Choose a reason for hiding this comment

williaster Apr 26, 2019

Choose a reason for hiding this comment

graceguo-supercat Apr 28, 2019

Choose a reason for hiding this comment

williaster Apr 26, 2019

Choose a reason for hiding this comment

williaster Apr 26, 2019

Choose a reason for hiding this comment

graceguo-supercat Apr 28, 2019 • edited Loading

Choose a reason for hiding this comment

williaster Apr 29, 2019

Choose a reason for hiding this comment

graceguo-supercat Apr 29, 2019

Choose a reason for hiding this comment

williaster Apr 26, 2019 • edited Loading

Choose a reason for hiding this comment

graceguo-supercat Apr 28, 2019

Choose a reason for hiding this comment

williaster left a comment

Choose a reason for hiding this comment

graceguo-supercat commented Apr 26, 2019 •

edited

Loading

codecov-io commented Apr 26, 2019 •

edited

Loading

graceguo-supercat Apr 28, 2019 •

edited

Loading

williaster Apr 26, 2019 •

edited

Loading