Skip to content

Conversation

@nytai
Copy link
Member

@nytai nytai commented Sep 24, 2019

CATEGORY

Choose one

  • Bug Fix
  • Enhancement (new features, refinement)
  • Refactor
  • Add tests
  • Build / Development Environment
  • Documentation

SUMMARY

dependent on: dpgaspar/Flask-AppBuilder#1116
Adds a new react menu component to the welcome view. The menu should be exactly the same as the one rendered by FAB.

The react menu can be added to other pages by overriding the navbar block and rendering nothing, then adding the menu component in the react layout.

TEST PLAN

the welcome view renders the same menu as any other view

ADDITIONAL INFORMATION

REVIEWERS

@nytai nytai force-pushed the tai/react-menu branch 4 times, most recently from 952aff4 to 617a3f0 Compare October 1, 2019 23:18
@codecov-io
Copy link

codecov-io commented Oct 1, 2019

Codecov Report

Merging #8289 into master will decrease coverage by 1.06%.
The diff coverage is 63.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8289      +/-   ##
==========================================
- Coverage   66.75%   65.69%   -1.07%     
==========================================
  Files         450      479      +29     
  Lines       22734    23641     +907     
  Branches     2366     2587     +221     
==========================================
+ Hits        15177    15530     +353     
- Misses       7419     7942     +523     
- Partials      138      169      +31
Impacted Files Coverage Δ
superset/views/core.py 72.06% <ø> (ø) ⬆️
superset/assets/src/welcome/App.jsx 0% <0%> (ø) ⬆️
superset/assets/src/components/Menu/MenuObject.jsx 20% <20%> (ø)
...rset/assets/src/components/Menu/LanguagePicker.jsx 33.33% <33.33%> (ø)
superset/assets/src/components/Menu/UserMenu.jsx 50% <50%> (ø)
superset/assets/src/components/Menu/NewMenu.jsx 60% <60%> (ø)
superset/views/base.py 73.18% <83.33%> (+0.63%) ⬆️
superset/assets/src/components/Menu/Menu.jsx 87.5% <87.5%> (ø)
superset/views/sql_lab.py 56.57% <0%> (-29.34%) ⬇️
.../assets/src/SqlLab/components/TabbedSqlEditors.jsx 74.81% <0%> (-8.23%) ⬇️
... and 72 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 59bc220...6bfee70. Read the comment docs.

@nytai nytai marked this pull request as ready for review October 2, 2019 16:32
@nytai nytai requested review from kristw and mistercrunch October 23, 2019 16:26
@mistercrunch mistercrunch changed the title react menu Move from MVC FAB menu to Superset React menu Oct 25, 2019
@mistercrunch
Copy link
Member

Should we go straight to do this on all templates? Progressive rollout per page?

@nytai
Copy link
Member Author

nytai commented Oct 25, 2019

@mistercrunch I was hoping for either a progressive rollout or a followup pr to replace the menu in all views. I'd like to avoid bloating this PR with a ton of files changed. Additionally if anything breaks in the new code the bug footprint will be limited to a single view.

Also, happy to do a full replace in this pr if you think that would be a better approach.

@mistercrunch mistercrunch merged commit 9d36fa3 into apache:master Nov 20, 2019
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.36.0 First shipped in 0.36.0 labels Feb 28, 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 size/XL 🚢 0.36.0 First shipped in 0.36.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get the navbar to be React-based and Superset owned instead of relying on FAB's MVC

5 participants