style: Fancier menus, more SIP-34-ish #10423
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #10423 +/- ##
==========================================
- Coverage 60.17% 60.01% -0.17%
==========================================
Files 784 785 +1
Lines 36954 36980 +26
Branches 3529 3530 +1
==========================================
- Hits 22238 22193 -45
- Misses 14529 14600 +71
Partials 187 187
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
mistercrunch
approved these changes
Jul 27, 2020
6 tasks
etr2460
reviewed
Jul 30, 2020
There was a problem hiding this comment.
My biggest question is about the !importants. Is there a way to avoid these? And if not, can we add comments explicitly saying why we need to use them?
To be clear, I would much, much rather we get rid of them. But if it's impossible, then a comment would help remove a bit of the code smell
|
Got the two lil' menus on the right to conform, for good measure. |
|
Addressed PR comments and made lots of touchups. @mistercrunch / @etr2460 if you wouldn't mind giving it another glance, that'd make me feel better before merging. @riahk / @nytai might want to take a glance too. |
|
Addresses aspects of #8976 |
mistercrunch
approved these changes
Aug 25, 2020
10 tasks
Ofeknielsen
pushed a commit
to ofekisr/incubator-superset
that referenced
this pull request
Oct 5, 2020
* style: shinier menus * fixing mouseover glitch * unused import * rm emotion-reset * restoring default config values * lint fixes ✨ * a bit more pizzaz to the underlines (max's idea), fading active background * simplifying navtitle -> label * RBNavDropdown -> ReactBootstrapNavDropdown * nixing whitespace * replacing !importants with better selector specificity * moving Menu LESS into Emotion * no more border! * fixing border issue * language picker, account dropdown now use new dropdown component * nixing whitespace in comment * nixing duplicate styling * removing borders on FAB navbar * explicit font coloring * linting
auxten
pushed a commit
to auxten/incubator-superset
that referenced
this pull request
Nov 20, 2020
* style: shinier menus * fixing mouseover glitch * unused import * rm emotion-reset * restoring default config values * lint fixes ✨ * a bit more pizzaz to the underlines (max's idea), fading active background * simplifying navtitle -> label * RBNavDropdown -> ReactBootstrapNavDropdown * nixing whitespace * replacing !importants with better selector specificity * moving Menu LESS into Emotion * no more border! * fixing border issue * language picker, account dropdown now use new dropdown component * nixing whitespace in comment * nixing duplicate styling * removing borders on FAB navbar * explicit font coloring * linting
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Moving the menus toward SIP-34. Design details are a bit up for grabs... I'll continue to steer with @Steejay. Probably best not to merge until #10401 is merged, so this doesn't cause headaches alternating between the two menus.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION