feat: implement secondary navigation for datasets #9982

lilykuang · 2020-06-03T22:39:03Z

SUMMARY

This PR is part of new UX/UI design for Superset, see SIP-34. It updates "Sources" navigation menu to new secondary navigation component below the main navigation. New navigation bar is behind feature LIST_VIEWS_NEW_UI and ENABLE_REACT_CRUD_VIEWS. The reset of the redesign is still working in process.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

After:

TEST PLAN

ADDITIONAL INFORMATION

codecov-commenter · 2020-06-03T22:46:26Z

Codecov Report

Merging #9982 into master will decrease coverage by 0.96%.
The diff coverage is 91.17%.

@@            Coverage Diff             @@
##           master    #9982      +/-   ##
==========================================
- Coverage   71.38%   70.42%   -0.97%     
==========================================
  Files         585      586       +1     
  Lines       30949    31072     +123     
  Branches     3261     3281      +20     
==========================================
- Hits        22094    21883     -211     
- Misses       8746     9078     +332     
- Partials      109      111       +2

Flag	Coverage Δ
#cypress	`53.92% <ø> (-0.16%)`	⬇️
#javascript	`59.44% <91.17%> (+0.04%)`	⬆️
#python	`69.98% <ø> (-1.60%)`	⬇️

Impacted Files	Coverage Δ
...set-frontend/src/views/datasetList/DatasetList.tsx	`55.55% <85.71%> (+0.92%)`	⬆️
superset-frontend/src/components/Menu/SubMenu.tsx	`92.59% <92.59%> (ø)`
superset/examples/countries.py	`0.00% <0.00%> (-100.00%)`	⬇️
superset/utils/decorators.py	`55.00% <0.00%> (-33.34%)`	⬇️
superset/viz.py	`57.15% <0.00%> (-14.83%)`	⬇️
...tend/src/explore/components/DisplayQueryButton.jsx	`68.42% <0.00%> (-7.90%)`	⬇️
superset-frontend/src/explore/exploreUtils.js	`74.45% <0.00%> (-7.20%)`	⬇️
...ponents/Select/WindowedSelect/WindowedMenuList.tsx	`93.10% <0.00%> (-3.45%)`	⬇️
superset/connectors/connector_registry.py	`84.44% <0.00%> (-2.23%)`	⬇️
...uperset-frontend/src/views/chartList/ChartList.tsx	`61.22% <0.00%> (-2.02%)`	⬇️
... and 51 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 dcac860...102981a. Read the comment docs.

mistercrunch · 2020-06-04T01:33:06Z

superset-frontend/src/components/Menu/Menu.less

+      text-transform: uppercase;
+      font-weight: 500;
+      font-size: @font-size-s;
+      background-color: #20a7c9;


Can we make the blue "+DATASET" button @brand-primary for the time being.

nytai

just a couple comments re: translation. otherwise looks good

superset-frontend/src/components/Menu/SubMenu.tsx

superset-frontend/src/views/datasetList/DatasetList.tsx

superset-frontend/src/components/Menu/SubMenu.tsx

willbarrett

Just one thought on color variables :)

willbarrett · 2020-06-04T16:59:33Z

superset-frontend/src/components/Menu/Menu.less

+        font-size: @font-size-s;
+        padding: 8px;
+        margin: 8px;
+        color: #3d3d3d;


Should this be a color variable?

seconded: if there is any chance these colors will be reused it should be moved into a variable

In cases of grays like this, there should be one pretty close to what you need... and we can adjust the global variable(s) to move closer to the new design as needed. Ping me if you want to discuss.

willbarrett · 2020-06-04T16:59:48Z

superset-frontend/src/components/Menu/Menu.less

+    }
+    li.active > a,
+    li > a:hover {
+      background: #eceef2;


Same question - should this be a variable?

mistercrunch · 2020-06-07T04:27:55Z

superset-frontend/stylesheets/less/variables.less

@@ -34,6 +34,8 @@
 @gray-bg: #f5f5f5;
 @gray-heading: #a3a3a3;
 @menu-hover: #f2f3f5;
+@gray-light-hover: #eceef2;
+@gray-drak-heading: #3d3d3d;


typo? should we just reuse one of the greys above?

ktmud · 2020-06-07T06:28:15Z

superset-frontend/src/components/Menu/Menu.less

+      border-radius: 4px;
+    }
+  }
+}


Hmm, should we move these to styled components or are we ok with living in a world with a mix of less + css-in-js?

@ktmud good suggestion. I think moving to these to styled components will be cleaner. I will move css to the component.

ktmud · 2020-06-07T06:34:56Z

superset-frontend/src/components/Menu/SubMenu.tsx

+ * under the License.
+ */
+import React from 'react';
+import { Button, Nav, Navbar, MenuItem } from 'react-bootstrap';


We may also want to decide whether to keep creating new UI components based on Bootstrap. To me, this seems like a great opportunity to migrate away from it.

What do you think, @rusackas @kristw ?

To me migrating away from bootstrap is an important decision with intricate considerations that are outside the scope of this PR. We should start a [DISCUSS] thread on the mailing list on this topic.

There were some discussion when SIP-34 was closed and I thought the general consensus was we wanted to at least move away from Bootstrap 3?

If we are to implement new UI components for the new design, it'd be the best to start clean and reduce dependencies on Bootstrap as much as possible, especially if the component doesn't need to reuse much styling from Bootstrap anyway.

implement secondary navigation for datasets

dd933bc

superset-github-bot bot added the preset-io label Jun 3, 2020

pull-request-size bot added the size/L label Jun 3, 2020

move create button to secondary navbar

21cbf89

lilykuang force-pushed the lily/secondary-navigation-datasets branch from 4dd33f7 to 21cbf89 Compare June 3, 2020 23:09

lilykuang marked this pull request as ready for review June 3, 2020 23:48

pkdotson approved these changes Jun 4, 2020

View reviewed changes

mistercrunch requested changes Jun 4, 2020

View reviewed changes

nytai reviewed Jun 4, 2020

View reviewed changes

lilykuang added 2 commits June 3, 2020 20:37

change button color to primary color

c1d48b5

update translation for dataset menu labels

88d3f46

lilykuang requested review from nytai and mistercrunch June 4, 2020 03:49

willbarrett reviewed Jun 4, 2020

View reviewed changes

update color variables

d97a519

mistercrunch reviewed Jun 7, 2020

View reviewed changes

ktmud reviewed Jun 7, 2020

View reviewed changes

lilykuang added 2 commits June 8, 2020 14:31

move css to subMenu component

04d7561

update datasets list test

102981a

lilykuang requested review from rusackas, mistercrunch and ktmud June 8, 2020 23:40

ktmud approved these changes Jun 10, 2020

View reviewed changes

nytai merged commit 5339d31 into apache:master Jun 10, 2020

nytai deleted the lily/secondary-navigation-datasets branch June 10, 2020 18:55

auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020

feat: implement secondary navigation for datasets (apache#9982)

b30f057

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement secondary navigation for datasets #9982

feat: implement secondary navigation for datasets #9982

lilykuang commented Jun 3, 2020 •

edited

codecov-commenter commented Jun 3, 2020 •

edited

mistercrunch Jun 4, 2020

nytai left a comment

willbarrett left a comment

willbarrett Jun 4, 2020

nytai Jun 4, 2020

rusackas Jun 8, 2020

willbarrett Jun 4, 2020

mistercrunch Jun 7, 2020

ktmud Jun 7, 2020

lilykuang Jun 8, 2020

ktmud Jun 7, 2020

mistercrunch Jun 8, 2020

ktmud Jun 10, 2020 •

edited

feat: implement secondary navigation for datasets #9982

feat: implement secondary navigation for datasets #9982

Conversation

lilykuang commented Jun 3, 2020 • edited

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

codecov-commenter commented Jun 3, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

nytai left a comment

Choose a reason for hiding this comment

willbarrett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktmud Jun 10, 2020 • edited

Choose a reason for hiding this comment

lilykuang commented Jun 3, 2020 •

edited

codecov-commenter commented Jun 3, 2020 •

edited

ktmud Jun 10, 2020 •

edited