Merge pull request #2388 from rhamilto/sg00dwin-vertical-nav-tab-issu…

…e-2372 Automatic merge from submit-queue. Rework nav to remove :hover selectors to address bugs, inconsistencies Fixes #2373 Building on what @sg00dwin did in https://github.com/sg00dwin/origin-web-console/tree/vertical-nav-tab-issue2373%23issuecomment-339359101, this removes all :hover styling changes in order to forge consistency across hover, click, and tab navigation. @sg00dwin, @spadgett, mind pulling these changes, testing, and seeing if everything is ok?
openshift · Oct 27, 2017 · 14188ec · 14188ec
2 parents 3d54f38 + 750a086
commit 14188ec
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 56 deletions.
diff --git a/app/scripts/directives/nav.js b/app/scripts/directives/nav.js
@@ -21,8 +21,8 @@ angular.module('openshiftConsole')
       templateUrl: 'views/_sidebar.html',
       controller: function($scope) {
         var path;
-        var hoverDelay = 300;
-        var hideDelay = hoverDelay + 200;
+        var hoverDelay = 200;
+        var hideDelay = hoverDelay + 100;
 
         $scope.navItems = Constants.PROJECT_NAVIGATION;
         $scope.sidebar = {};
@@ -89,6 +89,10 @@ angular.module('openshiftConsole')
         };
 
         $scope.itemClicked = function(primaryItem) {
+          // Remove `isHover` from any of the items if another primary item was
+          // activated using the keyboard.
+          clearHover();
+
           if (primaryItem.href) {
             // Make sure any secondary nav closes if a primary item with an
             // href was activated using the keyboard.
@@ -97,10 +101,6 @@ angular.module('openshiftConsole')
             return;
           }
 
-          // Remove `isHover` from any of the items if another primary item was
-          // activated using the keyboard.
-          clearHover();
-
           // Open the item regardless of whether the mouse is really over it
           // for keyboard and screen reader accessibility.
           primaryItem.isHover = true;
@@ -110,10 +110,6 @@ angular.module('openshiftConsole')
         };
 
         $scope.onMouseEnter = function(primaryItem) {
-          if (_.isEmpty(primaryItem.secondaryNavSections)) {
-            return;
-          }
-
           if (primaryItem.mouseLeaveTimeout) {
             $timeout.cancel(primaryItem.mouseLeaveTimeout);
             primaryItem.mouseLeaveTimeout = null;
@@ -122,15 +118,11 @@ angular.module('openshiftConsole')
           primaryItem.mouseEnterTimeout = $timeout(function() {
             primaryItem.isHover = true;
             primaryItem.mouseEnterTimeout = null;
-            $scope.sidebar.secondaryOpen = true;
+            $scope.sidebar.secondaryOpen = !_.isEmpty(primaryItem.secondaryNavSections);
           }, hoverDelay);
         };
 
         $scope.onMouseLeave = function(primaryItem) {
-          if (_.isEmpty(primaryItem.secondaryNavSections)) {
-            return;
-          }
-
           if (primaryItem.mouseEnterTimeout) {
             $timeout.cancel(primaryItem.mouseEnterTimeout);
             primaryItem.mouseEnterTimeout = null;

diff --git a/app/styles/_vertical-nav.less b/app/styles/_vertical-nav.less
@@ -11,7 +11,6 @@
     display: none;
   }
   .has-project-bar & {
-    transition: width .1s ease-in-out;
     top: (@navbar-os-header-height-mobile + @project-bar-height-mobile);
     @media(min-width: @screen-sm-min) {
       display: block;
@@ -29,29 +28,31 @@
       top: 0px;
       width: 100%;
     }
-    &.active > a,
-    &:focus > a,
-    &:hover > a,
-    > a:focus,
-    > a:hover {
-      background-color: @nav-pf-vertical-active-bg-color;
-      color: @nav-pf-vertical-active-color;
-      font-weight: 300;
-      &:after,
+    // override PatternFly :hover as we don't want :hover changes
+    &:hover > a {
+      background-color: transparent;
+      color: @nav-pf-vertical-color;
       .fa,
       .pficon {
-        color: @nav-pf-vertical-active-color;
+        color: @nav-pf-vertical-icon-color;
       }
     }
-    &.active > a {
+    > a:focus,
+    &.active > a,
+    &.is-hover > a {
+      background-color: @nav-pf-vertical-active-bg-color;
+      color: @nav-pf-vertical-active-color !important;
+      &:after {
+        color: @nav-pf-vertical-active-color !important;
+      }
       .fa,
       .pficon {
-        color: @nav-pf-vertical-active-icon-color !important;
+        color: @nav-pf-vertical-active-icon-color;
       }
     }
     > a {
       font-size: @font-size-base;
-      font-weight: 300;
+      font-weight: 300 !important;
       text-decoration: none;
       @media(max-width: @screen-xs-max) {
         height: auto;
@@ -70,23 +71,35 @@
         text-decoration: none !important;
       }
     }
-    // padding aligns arrow vertically
-    &.secondary-nav-item-pf > a:after {
-      padding: 16px 0;
-      @media(max-width: @screen-xs-max) {
-        padding: 8px 0;
+    &.secondary-nav-item-pf {
+      > a:after {
+        font-size: 14px; // make right arrow size consistent with project-bar arrows
+        padding: 16px 0; // padding aligns arrow vertically
+        @media(max-width: @screen-xs-max) {
+          padding: 8px 0;
+        }
+      }
+      &.is-hover > a {
+        width: @nav-pf-vertical-width + 1; // cover .nav-pf-vertical border-right
+        z-index: @zindex-navbar-fixed + 1; // raise z-index to overlay
+        &:after {
+          color: @nav-pf-vertical-active-color;
+          right: 21px; // move 1px to offset parent width 1px increase onHover due to pf rules
+        }
       }
-    }
-    // make right arrow size consistent with project-bar arrows
-    &.secondary-nav-item-pf > a:after {
-      font-size: 14px;
     }
   }
   &.collapsed {
-    .list-group-item.secondary-nav-item-pf:hover,
-    .list-group-item.secondary-nav-item-pf.active {
-      > a:after {
-        right: 12px !important; // prevent arrow moving onHover due to pf rules
+    .list-group-item.secondary-nav-item-pf {
+      &.is-hover > a {
+        width: @nav-pf-vertical-collapsed-width + 2; // cover .nav-pf-vertical.collapsed border-right
+      }
+      &.is-hover,
+      &:hover,
+      &.active {
+        > a:after {
+          right: 12px !important; // prevent arrow moving onHover due to pf rules
+        }
       }
     }
     .nav-pf-secondary-nav {

diff --git a/dist/scripts/scripts.js b/dist/scripts/scripts.js
@@ -10507,18 +10507,18 @@ resource: e.canI.resource,
 group: e.canI.group
 }, e.canI.verb, l.projectName));
 }, l.itemClicked = function(e) {
-if (e.href) return l.nav.showMobileNav = !1, void (l.sidebar.secondaryOpen = !1);
-m(), e.isHover = !0, e.mobileSecondary = l.isMobile, l.sidebar.showMobileSecondary = l.isMobile, l.sidebar.secondaryOpen = !0;
+if (m(), e.href) return l.nav.showMobileNav = !1, void (l.sidebar.secondaryOpen = !1);
+e.isHover = !0, e.mobileSecondary = l.isMobile, l.sidebar.showMobileSecondary = l.isMobile, l.sidebar.secondaryOpen = !0;
 }, l.onMouseEnter = function(e) {
-_.isEmpty(e.secondaryNavSections) || (e.mouseLeaveTimeout && (n.cancel(e.mouseLeaveTimeout), e.mouseLeaveTimeout = null), e.mouseEnterTimeout = n(function() {
-e.isHover = !0, e.mouseEnterTimeout = null, l.sidebar.secondaryOpen = !0;
-}, 300));
+e.mouseLeaveTimeout && (n.cancel(e.mouseLeaveTimeout), e.mouseLeaveTimeout = null), e.mouseEnterTimeout = n(function() {
+e.isHover = !0, e.mouseEnterTimeout = null, l.sidebar.secondaryOpen = !_.isEmpty(e.secondaryNavSections);
+}, 200);
 }, l.onMouseLeave = function(e) {
-_.isEmpty(e.secondaryNavSections) || (e.mouseEnterTimeout && (n.cancel(e.mouseEnterTimeout), e.mouseEnterTimeout = null), e.mouseLeaveTimeout = n(function() {
+e.mouseEnterTimeout && (n.cancel(e.mouseEnterTimeout), e.mouseEnterTimeout = null), e.mouseLeaveTimeout = n(function() {
 e.isHover = !1, e.mouseLeaveTimeout = null, l.sidebar.secondaryOpen = _.some(l.navItems, function(e) {
 return e.isHover && !_.isEmpty(e.secondaryNavSections);
 });
-}, 500));
+}, 300);
 }, l.closeNav = function() {
 m(), l.nav.showMobileNav = !1, l.sidebar.secondaryOpen = !1;
 }, l.collapseMobileSecondary = function(e, t) {

diff --git a/dist/styles/main.css b/dist/styles/main.css