-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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: Add home link to navbar #11851
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11851 +/- ##
==========================================
- Coverage 66.71% 63.71% -3.00%
==========================================
Files 917 917
Lines 44700 44735 +35
Branches 4235 4236 +1
==========================================
- Hits 29820 28502 -1318
- Misses 14762 16056 +1294
- Partials 118 177 +59
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
I'm curious, isn't this redundant when you consider the |
@etr2460 it's possible to override the URL and logo in Superset, so this provides an additional way to get to the Superset home page when that configuration is leveraged. |
@etr2460 this is in case someone has the |
@etr2460 |
Thanks for the clarification! Maybe we only show the new menu option if the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks for addressing my comment!
SUMMARY
This PR adds new element to the top navbar - HOME link redirecting to the homepage.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Navbar before:
Navbar after:
TEST PLAN
First, run command
superset init
to create default roles and permissions (navbar varies for different types of users, but home link should be seen by every user.) Then verify manually by visiting some application page.ADDITIONAL INFORMATION
cc @junlincc @rusackas