From bfdfc1165c5def0323d8d1f8800d7052fca436c3 Mon Sep 17 00:00:00 2001 From: Stephen Niedzielski Date: Mon, 5 Aug 2019 14:41:12 -0600 Subject: [PATCH] [UI] [menu] slide the main menu over the page Slide the main menu over the page instead of sliding the page over the menu. Also, use viewport units for the main and notification menus. Note, this lays foundation work for T225213. Bug: T206354 Change-Id: I14b67d1e97b84086ea13e28df8148824a1f493e3 --- minerva.less/minerva.variables.less | 2 - .../skins.minerva.base.styles/common.less | 1 + resources/skins.minerva.base.styles/ui.less | 13 +- .../MainMenu.less | 131 +++++------------- .../MainMenuItem.less | 5 + .../NotificationsOverlay.less | 18 ++- .../step_definitions/mainmenu_steps.rb | 2 +- .../features/step_definitions/menu_steps.js | 1 + 8 files changed, 51 insertions(+), 122 deletions(-) diff --git a/minerva.less/minerva.variables.less b/minerva.less/minerva.variables.less index 136d3f2..2294cfb 100644 --- a/minerva.less/minerva.variables.less +++ b/minerva.less/minerva.variables.less @@ -38,8 +38,6 @@ @skinContentBgColor: #fff; // Navigation Drawers -@menuWidth: 75%; -@menuWidthTablet: 600px; @rightDrawerWidth: 60%; @primaryNavBackgroundColor: @colorGray14; @menuButtonIconSize: 3.5em; diff --git a/resources/skins.minerva.base.styles/common.less b/resources/skins.minerva.base.styles/common.less index 1e95d14..58c3cd6 100644 --- a/resources/skins.minerva.base.styles/common.less +++ b/resources/skins.minerva.base.styles/common.less @@ -42,6 +42,7 @@ // overlap the menu during the main menu reveal/hide animation #mw-mf-page-center { width: 100%; + min-height: 100%; position: relative; background-color: @chromeColor; } diff --git a/resources/skins.minerva.base.styles/ui.less b/resources/skins.minerva.base.styles/ui.less index 19292dd..35c3414 100644 --- a/resources/skins.minerva.base.styles/ui.less +++ b/resources/skins.minerva.base.styles/ui.less @@ -269,19 +269,12 @@ input.search { } } -// This is here rather than in mainmenu.less because we want to load these rules for non-js users too -// Transparent shield hidden by default -.transparent-shield, -.navigation-drawer { - // don't use display: none because it's not animatable - position: absolute; - z-index: @z-indexBase; - visibility: hidden; -} - .transparent-shield { + position: absolute; background: @semiTransparent; z-index: @z-indexAboveContent; + // don't use display: none because it's not animatable + visibility: hidden; .transition( opacity 0.25s ease-in-out ); } diff --git a/resources/skins.minerva.mainMenu.styles/MainMenu.less b/resources/skins.minerva.mainMenu.styles/MainMenu.less index f2c92be..bfed28a 100644 --- a/resources/skins.minerva.mainMenu.styles/MainMenu.less +++ b/resources/skins.minerva.mainMenu.styles/MainMenu.less @@ -7,125 +7,58 @@ // .menu #mw-mf-page-left { - background-color: @primaryNavBackgroundColor; - float: left; - height: 100%; + position: fixed; + top: 0; + left: 0; + bottom: 0; + min-width: 275px; + + @media screen and ( min-width: @width-breakpoint-tablet ) { + min-width: @width-breakpoint-mobile; + } + // It should always be possible to dismiss the menu by tapping outside of it. + max-width: 80%; + z-index: @z-indexDrawer; // Add vertical scrollbar as needed. overflow-y: auto; + background-color: @primaryNavBackgroundColor; + .transform( translate( -100%, 0 ) ); ul { padding-bottom: 22px; } } -.navigation-enabled { - #mw-mf-viewport { - // disable horizontal scrolling - overflow: hidden; - // the two following properties are needed to override the height set - // by position: fixed fallback on scroll event - min-height: 100%; - } - - #mw-mf-page-center { - // Since we change the color of the body tag above we need to ensure the content has a white background - background: #fff; - position: absolute; - height: 100%; - // Overriden in mainmenuAnimation - left: @menuWidth; - } - - // .menu - #mw-mf-page-left, - .transparent-shield { - visibility: visible; - } - - .transparent-shield { - opacity: 0.5; - } -} -// FIXME: overrides the .has-drawer background color when a drawer is open. -// Should be removed when T214045 is resolved. -.primary-navigation-enabled.has-drawer { - background-color: @primaryNavBackgroundColor; -} - .primary-navigation-enabled { - background-color: @primaryNavBackgroundColor; - - // .menu - #mw-mf-page-left { - width: @menuWidth; - } - - // FIXME: Menu shouldn't need to know about drawers but a cta drawer might be open - .drawer .position-fixed { - left: @menuWidth !important; - } + overflow: hidden; + // Necessary to disable scrolling on Android and iOS devices. + touch-action: none; } -// FIXME: Cleanup animation css .animations { - #mw-mf-page-center { - min-height: 100%; - - // *2 to avoid weird glitch of left nav flickering after closing - @transition: @menu-animation-duration @menu-animation-easing, height 0s ( @menu-animation-duration * 2 ); - .transition-transform( @transition ); - } - // .menu #mw-mf-page-left { - width: @menuWidth; - - .transition( visibility 0s @menu-animation-duration ); - } -} - -.primary-navigation-enabled.animations { - // FIXME: Menu shouldn't need to know about drawers - .drawer .position-fixed, - #mw-mf-page-center { - // override non-animated version - left: 0 !important; - .transform( translate( @menuWidth, 0 ) ); + .transition( transform @menu-animation-duration @menu-animation-easing; ); } - // .menu - #mw-mf-page-left { - // make menu scrollable when open (on small screens) - position: static; - .transition( none ); - } -} - -@media all and ( min-width: @width-breakpoint-tablet ) { - .navigation-enabled { - // .menu - #mw-mf-page-center { - left: @menuWidthTablet; - } - } - - .primary-navigation-enabled { - &.animations { - // FIXME: Menu shouldn't need to know about drawers - .drawer .position-fixed, - #mw-mf-page-center { - .transform( translate( @menuWidthTablet, 0 ) ); - } - } - + &.primary-navigation-enabled { // .menu #mw-mf-page-left { - width: @menuWidthTablet; + .transform( translate( 0, 0 ) ); } - // FIXME: Menu shouldn't need to know about drawers but a cta drawer might be open - .drawer .position-fixed { - left: @menuWidthTablet !important; + .transparent-shield { + visibility: visible; + opacity: 0.5; } } } + +// Special:MobileMenu +.navigation-full-screen { + // .menu + #mw-mf-page-left { + max-width: none; + transform: none; + } +} diff --git a/resources/skins.minerva.mainMenu.styles/MainMenuItem.less b/resources/skins.minerva.mainMenu.styles/MainMenuItem.less index ab22f5f..69d4ca0 100644 --- a/resources/skins.minerva.mainMenu.styles/MainMenuItem.less +++ b/resources/skins.minerva.mainMenu.styles/MainMenuItem.less @@ -46,6 +46,11 @@ color: @colorGray5; display: block; padding: @menuLinkLineHeight / 2 10px @menuLinkLineHeight / 2 15px; + // Overflowing text is ellipsized. + max-width: 100%; + text-overflow: ellipsis; + white-space: nowrap; + overflow: hidden; &:hover { box-shadow: inset 4px 0 0 0 @colorProgressive; diff --git a/resources/skins.minerva.mainMenu.styles/NotificationsOverlay.less b/resources/skins.minerva.mainMenu.styles/NotificationsOverlay.less index f5079d2..9d5206f 100644 --- a/resources/skins.minerva.mainMenu.styles/NotificationsOverlay.less +++ b/resources/skins.minerva.mainMenu.styles/NotificationsOverlay.less @@ -9,8 +9,9 @@ // needs to be more specific than .overlay rules .notifications-overlay.navigation-drawer { - right: 0; + display: block; width: auto; + right: 0; box-shadow: -5px 0 0 0 rgba( 0, 0, 0, 0.3 ); } @@ -36,11 +37,8 @@ .animations { // FIXME: Make animations a conditional class on the drawer itself .notifications-overlay.navigation-drawer { - display: block; - - // +2% to accommodate for the border/box-shadow - .transform( translate( 102%, 0 ) ); - .transition-transform( @menu-animation-duration @menu-animation-easing; ); + .transform( translate( 100%, 0 ) ); + .transition( transform @menu-animation-duration @menu-animation-easing; ); &.visible { .transform( translate( 0, 0 ) ); @@ -48,9 +46,9 @@ } } -.secondary-navigation-enabled.animations { - #mw-mf-page-center { - // override non-animated version - left: 0 !important; +.secondary-navigation-enabled { + .transparent-shield { + visibility: visible; + opacity: 0.5; } } diff --git a/tests/browser/features/step_definitions/mainmenu_steps.rb b/tests/browser/features/step_definitions/mainmenu_steps.rb index 05c2e28..1c353ba 100644 --- a/tests/browser/features/step_definitions/mainmenu_steps.rb +++ b/tests/browser/features/step_definitions/mainmenu_steps.rb @@ -20,7 +20,7 @@ Then(/^I should see a link to the about page$/) do end Then(/^I should see a link to the disclaimer$/) do - expect(on(ArticlePage).disclaimer_link_element).to be_visible + expect(on(ArticlePage).disclaimer_link_element.when_visible).to be_visible end Then(/^I should see a link to my user page in the main navigation menu$/) do diff --git a/tests/selenium/features/step_definitions/menu_steps.js b/tests/selenium/features/step_definitions/menu_steps.js index df51736..4f5577a 100644 --- a/tests/selenium/features/step_definitions/menu_steps.js +++ b/tests/selenium/features/step_definitions/menu_steps.js @@ -19,6 +19,7 @@ const iShouldSeeALinkInMenu = ( text ) => { }; const iShouldSeeALinkToDisclaimer = () => { + ArticlePage.menu_element.element( '=Disclaimers' ).waitForVisible(); assert.strictEqual( ArticlePage.menu_element.element( '=Disclaimers' ).isVisible(), true ); };