fix: nav submenu dropdown styles #14580

pkdotson · 2021-05-11T21:13:03Z

SUMMARY

This pr removes the style overrides for the drop down submenus with moving the position of the anchor instead of overriding the inline styles.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2021-05-11.at.2.11.45.PM.mov

TEST PLAN

ADDITIONAL INFORMATION

Has associated issue:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

pkdotson · 2021-05-11T21:25:51Z

/testenv up

github-actions · 2021-05-11T21:26:30Z

@pkdotson Container image not yet published for this PR. Please try again when build is complete.

github-actions · 2021-05-11T21:26:31Z

@pkdotson Ephemeral environment creation failed. Please check the Actions logs for details.

codecov · 2021-05-11T21:33:16Z

Codecov Report

Merging #14580 (9391f23) into master (6d9d362) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #14580   +/-   ##
=======================================
  Coverage   77.53%   77.54%           
=======================================
  Files         958      958           
  Lines       48533    48526    -7     
  Branches     5703     5702    -1     
=======================================
- Hits        37631    37628    -3     
+ Misses      10701    10698    -3     
+ Partials      201      200    -1

Flag	Coverage Δ
javascript	`72.52% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset-frontend/src/common/components/index.tsx	`100.00% <100.00%> (ø)`
superset-frontend/src/components/Menu/Menu.tsx	`65.00% <100.00%> (+1.36%)`	⬆️
...nd/src/explore/components/ExploreViewContainer.jsx	`2.32% <0.00%> (-0.02%)`	⬇️
...uperset-frontend/src/components/Menu/MenuRight.tsx	`93.18% <0.00%> (ø)`
...nd/src/explore/components/ExploreActionButtons.tsx	`72.34% <0.00%> (ø)`
...ews/CRUD/annotationlayers/AnnotationLayersList.tsx	`77.35% <0.00%> (ø)`
...mponents/controls/MetricControl/MetricsControl.jsx	`82.45% <0.00%> (ø)`
...ontrols/FilterControl/AdhocFilterControl/index.jsx	`57.69% <0.00%> (ø)`
superset-frontend/src/components/Menu/SubMenu.tsx	`96.87% <0.00%> (+1.28%)`	⬆️
superset-frontend/src/components/Icons/Icon.tsx	`100.00% <0.00%> (+4.00%)`	⬆️
... and 1 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 6d9d362...9391f23. Read the comment docs.

junlincc · 2021-05-11T21:51:20Z

/testenv up

github-actions · 2021-05-11T21:53:25Z

@junlincc Ephemeral environment spinning up at http://34.213.183.152:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

superset-frontend/src/common/components/index.tsx

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

rusackas

Couple of small things to try, but this looks SO much better! Happy to see less LESS!

junlincc · 2021-05-12T02:43:18Z

lgtm thank you for the fix! 😊

… into fix-menu-styles

suddjian · 2021-05-13T22:25:55Z

/testenv up

github-actions · 2021-05-13T22:30:13Z

@suddjian Ephemeral environment spinning up at http://35.160.171.141:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

rusackas · 2021-05-14T06:01:53Z

Just needs a rebase... almost there.

michael-s-molina

Small nits. No blockers.

superset-frontend/src/common/components/index.tsx

michael-s-molina · 2021-05-14T17:03:57Z

superset-frontend/src/common/components/index.tsx

  & > .ant-menu-submenu-title {
    padding: 0 ${({ theme }) => theme.gridUnit * 6}px 0
      ${({ theme }) => theme.gridUnit * 3}px !important;
    svg {
      position: absolute;
-      top: ${({ theme }) => theme.gridUnit * 4}px;
+      top: ${({ theme }) => theme.gridUnit * 4 + 7}px;


Perhaps top: ${({ theme }) => theme.gridUnit * 5 + 3}px;?

This depends on what relationship we want the svg to have to the grid unit. When pixel amounts are hardcoded, it should be to format the thing being positioned so that it can look right in any grid. We could also do theme.gridUnit * 6 - 1, for example.

suddjian · 2021-05-14T18:58:31Z

I'm a little bit unclear on what this change is meant to fix. Is there a corresponding issue for this or a video of the problematic behavior?

pkdotson · 2021-05-14T20:06:45Z

I'm a little bit unclear on what this change is meant to fix. Is there a corresponding issue for this or a video of the problematic behavior?

@suddjian this is more of follow up to the navbar refactor that I merged. There were some body styles that needed to be removed from less to emotion. This is pr pretty much removes that and uses a css trick to get the dropdown to position correctly instead of overriding the inline styles from antd.

…ix-menu-styles

geido

LGTM just two nits

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

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

rusackas

LGTM

github-actions · 2021-05-17T20:58:24Z

Ephemeral environment shutdown and build artifacts deleted.

* fix nav submenu dropdown styles * lint * fix mobile view styles * run lint * address comments * undo comit lock files * Update superset-frontend/src/common/components/index.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> (cherry picked from commit 3ad8b54)

* fix nav submenu dropdown styles * lint * fix mobile view styles * run lint * address comments * undo comit lock files * Update superset-frontend/src/common/components/index.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

pkdotson added 4 commits May 11, 2021 12:41

fix nav submenu dropdown styles

6c939dd

lint

87e866e

fix mobile view styles

abbd85f

run lint

8283112

superset-github-bot bot added the preset-io label May 11, 2021

pull-request-size bot added the size/M label May 11, 2021

Merge branch 'master' into fix-menu-styles

9f64e43

pkdotson changed the title ~~Fix: nav submenu dropdown styles~~ fix: nav submenu dropdown styles May 11, 2021

junlincc added the bash! label May 11, 2021

rusackas requested review from suddjian and nytai May 11, 2021 23:42

rusackas reviewed May 11, 2021

View reviewed changes

superset-frontend/src/common/components/index.tsx Outdated Show resolved Hide resolved

rusackas reviewed May 11, 2021

View reviewed changes

superset-frontend/src/components/Menu/Menu.tsx Outdated Show resolved Hide resolved

rusackas reviewed May 11, 2021

View reviewed changes

junlincc added the need:review Requires a code review label May 13, 2021

pkdotson added 2 commits May 13, 2021 14:19

address comments

2df7684

Merge branch 'fix-menu-styles' of https://github.com/preset-io/superset…

8810bd7

… into fix-menu-styles

michael-s-molina reviewed May 14, 2021

View reviewed changes

Merge branch 'master' of https://github.com/preset-io/superset into f…

2f8b8f6

…ix-menu-styles

geido approved these changes May 17, 2021

View reviewed changes

superset-frontend/src/components/Menu/MenuRight.tsx Outdated Show resolved Hide resolved

superset-frontend/src/components/Menu/MenuRight.tsx Outdated Show resolved Hide resolved

undo comit lock files

f1c626c

pkdotson force-pushed the fix-menu-styles branch from e6a7a48 to f1c626c Compare May 17, 2021 18:35

Update superset-frontend/src/common/components/index.tsx

9391f23

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

rusackas approved these changes May 17, 2021

View reviewed changes

rusackas merged commit 3ad8b54 into apache:master May 17, 2021

rusackas deleted the fix-menu-styles branch May 17, 2021 20:58

junlincc added rush! Requires immediate attention and removed need:review Requires a code review labels May 18, 2021

junlincc removed the rush! Requires immediate attention label May 27, 2021

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: nav submenu dropdown styles #14580

fix: nav submenu dropdown styles #14580

pkdotson commented May 11, 2021 •

edited

pkdotson commented May 11, 2021

github-actions bot commented May 11, 2021

github-actions bot commented May 11, 2021

codecov bot commented May 11, 2021 •

edited

junlincc commented May 11, 2021

github-actions bot commented May 11, 2021

rusackas left a comment

junlincc commented May 12, 2021

suddjian commented May 13, 2021

github-actions bot commented May 13, 2021

rusackas commented May 14, 2021

michael-s-molina left a comment

michael-s-molina May 14, 2021

suddjian May 14, 2021

suddjian commented May 14, 2021

pkdotson commented May 14, 2021

geido left a comment

rusackas left a comment

github-actions bot commented May 17, 2021

fix: nav submenu dropdown styles #14580

fix: nav submenu dropdown styles #14580

Conversation

pkdotson commented May 11, 2021 • edited

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

pkdotson commented May 11, 2021

github-actions bot commented May 11, 2021

github-actions bot commented May 11, 2021

codecov bot commented May 11, 2021 • edited

Codecov Report

junlincc commented May 11, 2021

github-actions bot commented May 11, 2021

rusackas left a comment

Choose a reason for hiding this comment

junlincc commented May 12, 2021

suddjian commented May 13, 2021

github-actions bot commented May 13, 2021

rusackas commented May 14, 2021

michael-s-molina left a comment

Choose a reason for hiding this comment

michael-s-molina May 14, 2021

Choose a reason for hiding this comment

suddjian May 14, 2021

Choose a reason for hiding this comment

suddjian commented May 14, 2021

pkdotson commented May 14, 2021

geido left a comment

Choose a reason for hiding this comment

rusackas left a comment

Choose a reason for hiding this comment

github-actions bot commented May 17, 2021

pkdotson commented May 11, 2021 •

edited

codecov bot commented May 11, 2021 •

edited