From dab9b33f7cd84e30406bc5d5ff4efee89aee2a1c Mon Sep 17 00:00:00 2001 From: Carolyn MacLeod Date: Wed, 31 Jan 2018 19:53:35 -0500 Subject: [PATCH 1/8] Menubar examples: Make mouse behaviors and parent menu item appearance change triggers more consistent with desktop conventions #408 This commit fixes the menubar-2 example. It ensures the following behavior: - open menus stay open on mouseout - remove timeouts that close menu on mouseout - close menu when user: - clicks outside menu - clicks again on menubar item - clicks on menu item in menu - types tab or shift+tab - types space or enter on menu item (but not for space on radio/checkbox) - hovers over another menubar item (other menu opens) - menubar item retains its activated appearance while its menu is open Details of the code changes: - deleted hasFocus and hasHover state booleans because they are no longer needed - added popupMenu.isOpen() which checks for aria-expanded=true on the corresponding menubar item's DOM element - added ESC key handling on menubar items (closes menu if open) - clicking on menubar item opens menu, clicking again on same menubar item closes it - make sure browser doesn't scroll when clicking on menubar item (default behavior for clicking on link) - removed focus, blur, and mouseout event handling and timeouts from menubar items - removed focus, blur, mouseover, and mouseout event handling and timeouts from popup menu items - added focusout on menubar item and popup menu item to close menu if focus will be going outside of the menubar - if a menu is open, then hovering over a different menubar item opens its menu instead - removed check for anchor element children of menubar items (was probably an artefact from a previous implementation) - removed mouseover and mouseout events from popup menu - removed 'force' parameter from popup menu close() function - deleted some vars that were declared but not used, or were declared twice - fixed a few comments that didn't quite match the code - declared some vars as local that were declared as global (i.e. without var keyword) but did not need to be - removed some redundant checks, i.e. while (e) { if (e ...) } - changed != to !== and == to === where appropriate - removed css for hovering over menubar item (not needed because hovering focuses menubar item) --- .../menubar/menubar-2/css/menubarAction.css | 1 - .../menubar/menubar-2/js/MenubarAction.js | 51 ++++++------- .../menubar/menubar-2/js/MenubarItemAction.js | 75 ++++++++++--------- .../menubar/menubar-2/js/PopupMenuAction.js | 41 ++-------- .../menubar-2/js/PopupMenuItemAction.js | 50 +++++-------- examples/menubar/menubar-2/menubar-2.html | 2 +- 6 files changed, 86 insertions(+), 134 deletions(-) diff --git a/examples/menubar/menubar-2/css/menubarAction.css b/examples/menubar/menubar-2/css/menubarAction.css index b6b4b24a81..a042c243e7 100644 --- a/examples/menubar/menubar-2/css/menubarAction.css +++ b/examples/menubar/menubar-2/css/menubarAction.css @@ -69,7 +69,6 @@ ul[role="menubar"] [role="separator"] { } ul[role="menubar"] [role="menuitem"]:focus, -ul[role="menubar"] [role="menuitem"] [role="menuitem"]:hover, ul[role="menubar"] [role="menu"] [role="menuitem"]:hover, ul[role="menubar"] [role="menu"] [role="menuitemcheckbox"]:focus, ul[role="menubar"] [role="menu"] [role="menuitemcheckbox"]:hover, diff --git a/examples/menubar/menubar-2/js/MenubarAction.js b/examples/menubar/menubar-2/js/MenubarAction.js index 49a1db0532..4b3fe1530e 100644 --- a/examples/menubar/menubar-2/js/MenubarAction.js +++ b/examples/menubar/menubar-2/js/MenubarAction.js @@ -11,32 +11,32 @@ * @constructor MenubarAction * * @desc -* Wrapper object for a menubar (with nested submenus of links) +* Wrapper object for a menubar * * @param domNode -* The DOM element node that serves as the menubar container. Each -* child element of menubarNode that represents a menubaritem must -* be an A element +* The DOM element node that serves as the menubar container. +* Each child element of domNode that represents a menubaritem +* must be an A element. */ var MenubarAction = function (domNode) { - var elementChildren, - msgPrefix = 'Menubar constructor argument menubarNode '; + var msgPrefix = 'Menubar constructor argument domNode '; - // Check whether menubarNode is a DOM element + // Check whether domNode is a DOM element if (!domNode instanceof Element) { throw new TypeError(msgPrefix + 'is not a DOM Element.'); } - // Check whether menubarNode has descendant elements + // Check whether domNode has descendant elements if (domNode.childElementCount === 0) { throw new Error(msgPrefix + 'has no element children.'); } - // Check whether menubarNode has A elements - e = domNode.firstElementChild; + + // Check whether domNode's descendant elements contain A elements + var e = domNode.firstElementChild; while (e) { var menubarItem = e.firstElementChild; - if (e && menubarItem && menubarItem.tagName !== 'A') { - throw new Error(msgPrefix + 'has child elements are not A elements.'); + if (menubarItem && menubarItem.tagName !== 'A') { + throw new Error(msgPrefix + 'has child elements that are not A elements.'); } e = e.nextElementSibling; } @@ -48,9 +48,6 @@ var MenubarAction = function (domNode) { this.firstItem = null; // see Menubar init method this.lastItem = null; // see Menubar init method - - this.hasFocus = false; // see MenubarItem handleFocus, handleBlur - this.hasHover = false; // see Menubar handleMouseover, handleMouseout }; /* @@ -58,24 +55,24 @@ var MenubarAction = function (domNode) { * * @desc * Adds ARIA role to the menubar node -* Traverse menubar children for A elements to configure each A element as a ARIA menuitem +* Traverse menubar children for A elements to configure each A element as an ARIA menuitem * and populate menuitems array. Initialize firstItem and lastItem properties. */ MenubarAction.prototype.init = function (actionManager) { - var menubarItem, childElement, menuElement, textContent, numItems; + var menubarItem, menuElement, textContent, numItems; this.actionManager = actionManager; this.domNode.setAttribute('role', 'menubar'); - // Traverse the element children of menubarNode: configure each with + // Traverse the element children of the menubar domNode: configure each with // menuitem role behavior and store reference in menuitems array. - e = this.domNode.firstElementChild; + var e = this.domNode.firstElementChild; while (e) { - var menuElement = e.firstElementChild; + menuElement = e.firstElementChild; - if (e && menuElement && menuElement.tagName === 'A') { + if (menuElement && menuElement.tagName === 'A') { menubarItem = new MenubarItemAction(menuElement, this); menubarItem.init(); this.menubarItems.push(menubarItem); @@ -99,11 +96,10 @@ MenubarAction.prototype.init = function (actionManager) { MenubarAction.prototype.setFocusToItem = function (newItem) { var flag = false; - var newItem; for (var i = 0; i < this.menubarItems.length; i++) { var mbi = this.menubarItems[i]; - if (mbi.domNode.tabIndex == 0) { - flag = mbi.domNode.getAttribute('aria-expanded') === 'true'; + if (mbi.domNode.tabIndex === 0) { + flag = mbi.popupMenu && mbi.popupMenu.isOpen(); } mbi.domNode.tabIndex = -1; if (mbi.popupMenu) { @@ -124,8 +120,7 @@ MenubarAction.prototype.setFocusToLastItem = function (flag) { }; MenubarAction.prototype.setFocusToPreviousItem = function (currentItem) { - - var index; + var newItem, index; if (currentItem === this.firstItem) { newItem = this.lastItem; @@ -136,11 +131,10 @@ MenubarAction.prototype.setFocusToPreviousItem = function (currentItem) { } this.setFocusToItem(newItem); - }; MenubarAction.prototype.setFocusToNextItem = function (currentItem) { - var index; + var newItem, index; if (currentItem === this.lastItem) { newItem = this.firstItem; @@ -150,7 +144,6 @@ MenubarAction.prototype.setFocusToNextItem = function (currentItem) { newItem = this.menubarItems[ index + 1 ]; } this.setFocusToItem(newItem); - }; MenubarAction.prototype.setFocusByFirstCharacter = function (currentItem, char) { diff --git a/examples/menubar/menubar-2/js/MenubarItemAction.js b/examples/menubar/menubar-2/js/MenubarItemAction.js index 12006ee1c5..b1e1bcd3b4 100644 --- a/examples/menubar/menubar-2/js/MenubarItemAction.js +++ b/examples/menubar/menubar-2/js/MenubarItemAction.js @@ -8,38 +8,27 @@ */ /* -* @constructor MenubarItem +* @constructor MenubarItemAction * * @desc -* Object that configures menu item elements by setting tabIndex +* Object that configures menubar item elements by setting tabIndex * and registering itself to handle pertinent events. * -* While menuitem elements handle many keydown events, as well as -* focus and blur events, they do not maintain any state variables, -* delegating those responsibilities to its associated menu object. -* -* Consequently, it is only necessary to create one instance of -* MenubarItem from within the menu object; its configure method -* can then be called on each menuitem element. -* * @param domNode -* The DOM element node that serves as the menu item container. -* The menuObj PopupMenu is responsible for checking that it has +* The DOM element node that serves as the menubar item container. +* The menubarObj is responsible for checking that it has * requisite metadata, e.g. role="menuitem". * -* @param menuObj -* The PopupMenu object that is a delegate for the menu DOM element -* that contains the menuitem element. +* @param menubarObj +* The MenubarAction object that is a delegate for the menubar DOM element +* that contains the menubar item element. */ -var MenubarItemAction = function (domNode, menuObj) { +var MenubarItemAction = function (domNode, menubarObj) { - this.menubar = menuObj; + this.menubar = menubarObj; this.domNode = domNode; this.popupMenu = false; - this.hasFocus = false; - this.hasHover = false; - this.keyCode = Object.freeze({ 'TAB': 9, 'RETURN': 13, @@ -61,10 +50,8 @@ MenubarItemAction.prototype.init = function () { this.domNode.addEventListener('keydown', this.handleKeydown.bind(this)); this.domNode.addEventListener('click', this.handleClick.bind(this)); - this.domNode.addEventListener('focus', this.handleFocus.bind(this)); - this.domNode.addEventListener('blur', this.handleBlur.bind(this)); + this.domNode.addEventListener('focusout', this.handleFocusout.bind(this)); this.domNode.addEventListener('mouseover', this.handleMouseover.bind(this)); - this.domNode.addEventListener('mouseout', this.handleMouseout.bind(this)); // initialize pop up menus @@ -80,8 +67,7 @@ MenubarItemAction.prototype.init = function () { MenubarItemAction.prototype.handleKeydown = function (event) { var tgt = event.currentTarget, char = event.key, - flag = false, - clickEvent; + flag = false; function isPrintableCharacter (str) { return str.length === 1 && str.match(/\S/); @@ -98,6 +84,13 @@ MenubarItemAction.prototype.handleKeydown = function (event) { } break; + case this.keyCode.ESC: + if (this.popupMenu) { + this.popupMenu.close(); + } + flag = true; + break; + case this.keyCode.LEFT: this.menubar.setFocusToPreviousItem(this); flag = true; @@ -149,22 +142,32 @@ MenubarItemAction.prototype.handleKeydown = function (event) { }; MenubarItemAction.prototype.handleClick = function (event) { + if (this.popupMenu) { + if (!this.popupMenu.isOpen()) { + // clicking on menubar item opens menu + this.popupMenu.open(); + } else { + // clicking again on same menubar item closes menu + this.popupMenu.close(); + } + // prevent scroll to top of page when anchor element is clicked + event.preventDefault(); + } }; -MenubarItemAction.prototype.handleFocus = function (event) { - this.menubar.hasFocus = true; +MenubarItemAction.prototype.menubarElement = function (el) { + return this.menubar.domNode.contains(el); }; -MenubarItemAction.prototype.handleBlur = function (event) { - this.menubar.hasFocus = false; +MenubarItemAction.prototype.handleFocusout = function (event) { + // if the next element to get focus is not in the menubar or its menus, then close menu + if (!this.menubarElement(event.relatedTarget)) { + if (this.popupMenu && this.popupMenu.isOpen()) { + this.popupMenu.close(); + } + } }; MenubarItemAction.prototype.handleMouseover = function (event) { - this.hasHover = true; - this.popupMenu.open(); -}; - -MenubarItemAction.prototype.handleMouseout = function (event) { - this.hasHover = false; - setTimeout(this.popupMenu.close.bind(this.popupMenu, false), 300); + this.menubar.setFocusToItem(this); }; diff --git a/examples/menubar/menubar-2/js/PopupMenuAction.js b/examples/menubar/menubar-2/js/PopupMenuAction.js index 0814bf46e1..c0ec916713 100644 --- a/examples/menubar/menubar-2/js/PopupMenuAction.js +++ b/examples/menubar/menubar-2/js/PopupMenuAction.js @@ -26,9 +26,6 @@ * The controller object is expected to have the following properties: * 1. domNode: The controller object's DOM element node, needed for * retrieving positioning information. -* 2. hasHover: boolean that indicates whether the controller object's -* domNode has responded to a mouseover event with no subsequent -* mouseout event having occurred. */ var PopupMenuAction = function (domNode, controllerObj, actionManager) { var elementChildren, @@ -44,16 +41,6 @@ var PopupMenuAction = function (domNode, controllerObj, actionManager) { throw new Error(msgPrefix + 'has no element children.'); } - // Check whether domNode descendant elements have A elements - var childElement = domNode.firstElementChild; - while (childElement) { - var menuitem = childElement.firstElementChild; - if (menuitem && menuitem === 'A') { - throw new Error(msgPrefix + 'has descendant elements that are not A elements.'); - } - childElement = childElement.nextElementSibling; - } - this.domNode = domNode; this.controller = controllerObj; this.actionManager = actionManager; @@ -63,9 +50,6 @@ var PopupMenuAction = function (domNode, controllerObj, actionManager) { this.firstItem = null; // see PopupMenu init method this.lastItem = null; // see PopupMenu init method - - this.hasFocus = false; // see MenuItem handleFocus, handleBlur - this.hasHover = false; // see PopupMenu handleMouseover, handleMouseout }; /* @@ -89,9 +73,6 @@ PopupMenuAction.prototype.init = function () { this.domNode.setAttribute('aria-label', label); } - this.domNode.addEventListener('mouseover', this.handleMouseover.bind(this)); - this.domNode.addEventListener('mouseout', this.handleMouseout.bind(this)); - // Traverse the element children of domNode: configure each with // menuitem role behavior and store reference in menuitems array. menuElements = this.domNode.getElementsByTagName('LI'); @@ -100,7 +81,7 @@ PopupMenuAction.prototype.init = function () { menuElement = menuElements[i]; - if (!menuElement.firstElementChild && menuElement.getAttribute('role') != 'separator') { + if (!menuElement.firstElementChild && menuElement.getAttribute('role') !== 'separator') { menuItem = new MenuItem(menuElement, this); menuItem.init(); this.menuitems.push(menuItem); @@ -158,17 +139,6 @@ PopupMenuAction.prototype.updateMenuStates = function () { }; -/* EVENT HANDLERS */ - -PopupMenuAction.prototype.handleMouseover = function (event) { - this.hasHover = true; -}; - -PopupMenuAction.prototype.handleMouseout = function (event) { - this.hasHover = false; - setTimeout(this.close.bind(this, false), 300); -}; - /* FOCUS MANAGEMENT METHODS */ PopupMenuAction.prototype.setFocusToController = function (command) { @@ -257,21 +227,22 @@ PopupMenuAction.prototype.open = function () { var rect = this.controller.domNode.getBoundingClientRect(); // set CSS properties - this.domNode.style.display = 'block'; this.domNode.style.position = 'absolute'; this.domNode.style.top = (rect.height - 1) + 'px'; this.domNode.style.left = '0px'; this.domNode.style.zIndex = 100; + this.domNode.style.display = 'block'; // set aria-expanded attribute this.controller.domNode.setAttribute('aria-expanded', 'true'); }; -PopupMenuAction.prototype.close = function (force) { +PopupMenuAction.prototype.isOpen = function () { + return this.controller.domNode.getAttribute('aria-expanded') === 'true'; +}; - if (force || (!this.hasFocus && !this.hasHover && !this.controller.hasHover)) { +PopupMenuAction.prototype.close = function () { this.domNode.style.display = 'none'; this.domNode.style.zIndex = 0; this.controller.domNode.setAttribute('aria-expanded', 'false'); - } }; diff --git a/examples/menubar/menubar-2/js/PopupMenuItemAction.js b/examples/menubar/menubar-2/js/PopupMenuItemAction.js index 7b0f6bab6f..a18a176508 100644 --- a/examples/menubar/menubar-2/js/PopupMenuItemAction.js +++ b/examples/menubar/menubar-2/js/PopupMenuItemAction.js @@ -52,10 +52,7 @@ MenuItem.prototype.init = function () { this.domNode.addEventListener('keydown', this.handleKeydown.bind(this)); this.domNode.addEventListener('click', this.handleClick.bind(this)); - this.domNode.addEventListener('focus', this.handleFocus.bind(this)); - this.domNode.addEventListener('blur', this.handleBlur.bind(this)); - this.domNode.addEventListener('mouseover', this.handleMouseover.bind(this)); - this.domNode.addEventListener('mouseout', this.handleMouseout.bind(this)); + this.domNode.addEventListener('focusout', this.handleFocusout.bind(this)); }; @@ -66,6 +63,7 @@ MenuItem.prototype.activateMenuitem = function (node) { var option = node.getAttribute('rel'); var item; // flag is used to signal whether a menu should close or not + // i.e. don't close if checkbox or radio menuitem is toggled var flag = true; if (typeof option !== 'string') { @@ -77,7 +75,7 @@ MenuItem.prototype.activateMenuitem = function (node) { } else { if (role === 'menuitemcheckbox') { - if (node.getAttribute('aria-checked') == 'true') { + if (node.getAttribute('aria-checked') === 'true') { this.menu.actionManager.setOption(option, false); node.setAttribute('aria-checked', 'false'); } @@ -116,8 +114,7 @@ MenuItem.prototype.activateMenuitem = function (node) { MenuItem.prototype.handleKeydown = function (event) { var tgt = event.currentTarget, char = event.key, - flag = false, - clickEvent; + flag = false; function isPrintableCharacter (str) { return str.length === 1 && str.match(/\S/); @@ -127,7 +124,7 @@ MenuItem.prototype.handleKeydown = function (event) { case this.keyCode.SPACE: if (this.activateMenuitem(tgt)) { this.menu.setFocusToController(); - this.menu.close(true); + this.menu.close(); } flag = true; break; @@ -135,13 +132,13 @@ MenuItem.prototype.handleKeydown = function (event) { case this.keyCode.RETURN: this.activateMenuitem(tgt); this.menu.setFocusToController(); - this.menu.close(true); + this.menu.close(); flag = true; break; case this.keyCode.ESC: this.menu.setFocusToController(); - this.menu.close(true); + this.menu.close(); flag = true; break; @@ -157,13 +154,13 @@ MenuItem.prototype.handleKeydown = function (event) { case this.keyCode.LEFT: this.menu.setFocusToController('previous'); - this.menu.close(true); + this.menu.close(); flag = true; break; case this.keyCode.RIGHT: this.menu.setFocusToController('next'); - this.menu.close(true); + this.menu.close(); flag = true; break; @@ -181,7 +178,8 @@ MenuItem.prototype.handleKeydown = function (event) { case this.keyCode.TAB: this.menu.setFocusToController(); - this.menu.close(true); + this.menu.close(); + // allow tab and shift+tab to navigate out of menu bar break; default: @@ -201,25 +199,13 @@ MenuItem.prototype.handleKeydown = function (event) { MenuItem.prototype.handleClick = function (event) { this.activateMenuitem(event.currentTarget); this.menu.setFocusToController(); - this.menu.close(true); + this.menu.close(); }; -MenuItem.prototype.handleFocus = function (event) { - this.menu.hasFocus = true; -}; - -MenuItem.prototype.handleBlur = function (event) { - this.menu.hasFocus = false; - setTimeout(this.menu.close.bind(this.menu, false), 300); -}; - -MenuItem.prototype.handleMouseover = function (event) { - this.menu.hasHover = true; - this.menu.open(); - -}; - -MenuItem.prototype.handleMouseout = function (event) { - this.menu.hasHover = false; - setTimeout(this.menu.close.bind(this.menu, false), 300); +MenuItem.prototype.handleFocusout = function (event) { + // if the next element to get focus is not in the menubar or its menus, then close menu + if (!this.menu.controller.menubarElement(event.relatedTarget)) { + this.menu.close(); + } + console.log(); }; diff --git a/examples/menubar/menubar-2/menubar-2.html b/examples/menubar/menubar-2/menubar-2.html index 8016a351c4..e60a4ecc92 100644 --- a/examples/menubar/menubar-2/menubar-2.html +++ b/examples/menubar/menubar-2/menubar-2.html @@ -439,7 +439,7 @@

Menubar

li - Indicates the submenu is open. + Indicates the menu is open. From 1450041cec18cc7e11dc243037064397b12b5dd4 Mon Sep 17 00:00:00 2001 From: Carolyn MacLeod Date: Fri, 2 Feb 2018 09:11:24 -0500 Subject: [PATCH 2/8] Fix code style problems flagged by Travis CI issue #408 --- .../menubar/menubar-2/js/MenubarItemAction.js | 15 ++++++++------- examples/menubar/menubar-2/js/PopupMenuAction.js | 8 ++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/examples/menubar/menubar-2/js/MenubarItemAction.js b/examples/menubar/menubar-2/js/MenubarItemAction.js index b1e1bcd3b4..ef3ee32d15 100644 --- a/examples/menubar/menubar-2/js/MenubarItemAction.js +++ b/examples/menubar/menubar-2/js/MenubarItemAction.js @@ -144,11 +144,12 @@ MenubarItemAction.prototype.handleKeydown = function (event) { MenubarItemAction.prototype.handleClick = function (event) { if (this.popupMenu) { if (!this.popupMenu.isOpen()) { - // clicking on menubar item opens menu - this.popupMenu.open(); - } else { - // clicking again on same menubar item closes menu - this.popupMenu.close(); + // clicking on menubar item opens menu + this.popupMenu.open(); + } + else { + // clicking again on same menubar item closes menu + this.popupMenu.close(); } // prevent scroll to top of page when anchor element is clicked event.preventDefault(); @@ -156,14 +157,14 @@ MenubarItemAction.prototype.handleClick = function (event) { }; MenubarItemAction.prototype.menubarElement = function (el) { - return this.menubar.domNode.contains(el); + return this.menubar.domNode.contains(el); }; MenubarItemAction.prototype.handleFocusout = function (event) { // if the next element to get focus is not in the menubar or its menus, then close menu if (!this.menubarElement(event.relatedTarget)) { if (this.popupMenu && this.popupMenu.isOpen()) { - this.popupMenu.close(); + this.popupMenu.close(); } } }; diff --git a/examples/menubar/menubar-2/js/PopupMenuAction.js b/examples/menubar/menubar-2/js/PopupMenuAction.js index c0ec916713..ec3a721384 100644 --- a/examples/menubar/menubar-2/js/PopupMenuAction.js +++ b/examples/menubar/menubar-2/js/PopupMenuAction.js @@ -238,11 +238,11 @@ PopupMenuAction.prototype.open = function () { }; PopupMenuAction.prototype.isOpen = function () { - return this.controller.domNode.getAttribute('aria-expanded') === 'true'; + return this.controller.domNode.getAttribute('aria-expanded') === 'true'; }; PopupMenuAction.prototype.close = function () { - this.domNode.style.display = 'none'; - this.domNode.style.zIndex = 0; - this.controller.domNode.setAttribute('aria-expanded', 'false'); + this.domNode.style.display = 'none'; + this.domNode.style.zIndex = 0; + this.controller.domNode.setAttribute('aria-expanded', 'false'); }; From 8ec46127e2fceac7ea0b054bdef4a5855d437994 Mon Sep 17 00:00:00 2001 From: Carolyn MacLeod Date: Tue, 20 Feb 2018 23:45:35 -0500 Subject: [PATCH 3/8] Additional fix for issue #408: hovering over a menubar item does not give it focus (unless the menubar already has focus - see issue #408 comment for explanation) Details of change: - added "focus" class on the menubar as a styling hook to not show hover style if menubar is focussed - added focusin handling to menubar, and moved focusout handling up to menubar (formerly in menubar item and menu item) to support .focus styling hook - changed menubar's setFocusToItem function to not change focus or tabindex on mouse hover, unless menubar already has focus - added mouseover handling to menuitem to give focus to hovered item in open menu - removed unused flag parameter from 2 functions --- .../menubar/menubar-2/css/menubarAction.css | 2 + .../menubar/menubar-2/js/MenubarAction.js | 47 +++++++++++++++---- .../menubar/menubar-2/js/MenubarItemAction.js | 16 +------ .../menubar/menubar-2/js/PopupMenuAction.js | 4 ++ .../menubar-2/js/PopupMenuItemAction.js | 13 ++--- 5 files changed, 48 insertions(+), 34 deletions(-) diff --git a/examples/menubar/menubar-2/css/menubarAction.css b/examples/menubar/menubar-2/css/menubarAction.css index a042c243e7..a26fbc334e 100644 --- a/examples/menubar/menubar-2/css/menubarAction.css +++ b/examples/menubar/menubar-2/css/menubarAction.css @@ -69,6 +69,8 @@ ul[role="menubar"] [role="separator"] { } ul[role="menubar"] [role="menuitem"]:focus, +ul[role="menubar"]:not(.focus) [role="menuitem"]:hover, +ul[role="menubar"] [role="menu"] [role="menuitem"]:focus, ul[role="menubar"] [role="menu"] [role="menuitem"]:hover, ul[role="menubar"] [role="menu"] [role="menuitemcheckbox"]:focus, ul[role="menubar"] [role="menu"] [role="menuitemcheckbox"]:hover, diff --git a/examples/menubar/menubar-2/js/MenubarAction.js b/examples/menubar/menubar-2/js/MenubarAction.js index 4b3fe1530e..bca6f50e4d 100644 --- a/examples/menubar/menubar-2/js/MenubarAction.js +++ b/examples/menubar/menubar-2/js/MenubarAction.js @@ -65,6 +65,9 @@ MenubarAction.prototype.init = function (actionManager) { this.domNode.setAttribute('role', 'menubar'); + this.domNode.addEventListener('focusin', this.handleFocusin.bind(this)); + this.domNode.addEventListener('focusout', this.handleFocusout.bind(this)); + // Traverse the element children of the menubar domNode: configure each with // menuitem role behavior and store reference in menuitems array. var e = this.domNode.firstElementChild; @@ -94,28 +97,32 @@ MenubarAction.prototype.init = function (actionManager) { /* FOCUS MANAGEMENT METHODS */ -MenubarAction.prototype.setFocusToItem = function (newItem) { - var flag = false; +MenubarAction.prototype.setFocusToItem = function (newItem, hover) { + var isOpen = false; + var hasFocus = this.domNode.contains(document.activeElement); for (var i = 0; i < this.menubarItems.length; i++) { var mbi = this.menubarItems[i]; - if (mbi.domNode.tabIndex === 0) { - flag = mbi.popupMenu && mbi.popupMenu.isOpen(); + isOpen = isOpen || (mbi.popupMenu && mbi.popupMenu.isOpen()); + if (!hover || hasFocus) { + mbi.domNode.tabIndex = -1; } - mbi.domNode.tabIndex = -1; if (mbi.popupMenu) { mbi.popupMenu.close(); } } - newItem.domNode.focus(); - newItem.domNode.tabIndex = 0; - if (flag && newItem.popupMenu) { + if (!hover || hasFocus) { + newItem.domNode.focus(); + newItem.domNode.tabIndex = 0; + } + if (isOpen && newItem.popupMenu) { newItem.popupMenu.open(); } }; -MenubarAction.prototype.setFocusToFirstItem = function (flag) { + +MenubarAction.prototype.setFocusToFirstItem = function () { this.setFocusToItem(this.firstItem); }; -MenubarAction.prototype.setFocusToLastItem = function (flag) { +MenubarAction.prototype.setFocusToLastItem = function () { this.setFocusToItem(this.lastItem); }; @@ -179,3 +186,23 @@ MenubarAction.prototype.getIndexFirstChars = function (startIndex, char) { } return -1; }; + +MenubarAction.prototype.handleFocusin = function (event) { + // if the menubar or any of its menus has focus, add styling hook for hover + this.domNode.classList.add('focus'); +}; + +MenubarAction.prototype.handleFocusout = function (event) { + // if the next element to get focus is not in the menubar or its menus, then close menu + if (!this.domNode.contains(event.relatedTarget)) { + for (var i = 0; i < this.menubarItems.length; i++) { + var mbi = this.menubarItems[i]; + if (mbi.popupMenu && mbi.popupMenu.isOpen()) { + mbi.popupMenu.close(); + } + } + } + // remove styling hook for hover on menubar item + this.domNode.classList.remove('focus'); +}; + diff --git a/examples/menubar/menubar-2/js/MenubarItemAction.js b/examples/menubar/menubar-2/js/MenubarItemAction.js index ef3ee32d15..46837c0f69 100644 --- a/examples/menubar/menubar-2/js/MenubarItemAction.js +++ b/examples/menubar/menubar-2/js/MenubarItemAction.js @@ -50,7 +50,6 @@ MenubarItemAction.prototype.init = function () { this.domNode.addEventListener('keydown', this.handleKeydown.bind(this)); this.domNode.addEventListener('click', this.handleClick.bind(this)); - this.domNode.addEventListener('focusout', this.handleFocusout.bind(this)); this.domNode.addEventListener('mouseover', this.handleMouseover.bind(this)); // initialize pop up menus @@ -156,19 +155,6 @@ MenubarItemAction.prototype.handleClick = function (event) { } }; -MenubarItemAction.prototype.menubarElement = function (el) { - return this.menubar.domNode.contains(el); -}; - -MenubarItemAction.prototype.handleFocusout = function (event) { - // if the next element to get focus is not in the menubar or its menus, then close menu - if (!this.menubarElement(event.relatedTarget)) { - if (this.popupMenu && this.popupMenu.isOpen()) { - this.popupMenu.close(); - } - } -}; - MenubarItemAction.prototype.handleMouseover = function (event) { - this.menubar.setFocusToItem(this); + this.menubar.setFocusToItem(this, true); }; diff --git a/examples/menubar/menubar-2/js/PopupMenuAction.js b/examples/menubar/menubar-2/js/PopupMenuAction.js index ec3a721384..b8824dadde 100644 --- a/examples/menubar/menubar-2/js/PopupMenuAction.js +++ b/examples/menubar/menubar-2/js/PopupMenuAction.js @@ -156,6 +156,10 @@ PopupMenuAction.prototype.setFocusToController = function (command) { } }; +PopupMenuAction.prototype.setFocusToItem = function (item) { + item.domNode.focus(); +}; + PopupMenuAction.prototype.setFocusToFirstItem = function () { this.firstItem.domNode.focus(); }; diff --git a/examples/menubar/menubar-2/js/PopupMenuItemAction.js b/examples/menubar/menubar-2/js/PopupMenuItemAction.js index a18a176508..343086a36f 100644 --- a/examples/menubar/menubar-2/js/PopupMenuItemAction.js +++ b/examples/menubar/menubar-2/js/PopupMenuItemAction.js @@ -52,8 +52,7 @@ MenuItem.prototype.init = function () { this.domNode.addEventListener('keydown', this.handleKeydown.bind(this)); this.domNode.addEventListener('click', this.handleClick.bind(this)); - this.domNode.addEventListener('focusout', this.handleFocusout.bind(this)); - + this.domNode.addEventListener('mouseover', this.handleMouseover.bind(this)); }; MenuItem.prototype.activateMenuitem = function (node) { @@ -202,10 +201,6 @@ MenuItem.prototype.handleClick = function (event) { this.menu.close(); }; -MenuItem.prototype.handleFocusout = function (event) { - // if the next element to get focus is not in the menubar or its menus, then close menu - if (!this.menu.controller.menubarElement(event.relatedTarget)) { - this.menu.close(); - } - console.log(); -}; +MenuItem.prototype.handleMouseover = function (event) { + this.menu.setFocusToItem(this); +}; \ No newline at end of file From 9bb95d6719357372e523faf7a770cd145bd94a6b Mon Sep 17 00:00:00 2001 From: Carolyn MacLeod Date: Wed, 21 Feb 2018 10:50:08 -0500 Subject: [PATCH 4/8] Bug fix: if mouse is over menubar item and keyboard left/right opens another menu, then mouse click without moving first should close open menu first before opening. issue #408 --- examples/menubar/menubar-2/js/MenubarItemAction.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/examples/menubar/menubar-2/js/MenubarItemAction.js b/examples/menubar/menubar-2/js/MenubarItemAction.js index 46837c0f69..9a6ab78e2f 100644 --- a/examples/menubar/menubar-2/js/MenubarItemAction.js +++ b/examples/menubar/menubar-2/js/MenubarItemAction.js @@ -143,7 +143,14 @@ MenubarItemAction.prototype.handleKeydown = function (event) { MenubarItemAction.prototype.handleClick = function (event) { if (this.popupMenu) { if (!this.popupMenu.isOpen()) { - // clicking on menubar item opens menu + // clicking on menubar item opens menu (closes open menu first) + for (var i = 0; i < this.menubar.menubarItems.length; i++) { + var mbi = this.menubar.menubarItems[i]; + if (mbi.popupMenu && mbi.popupMenu.isOpen()) { + mbi.popupMenu.close(); + break; + } + } this.popupMenu.open(); } else { From 8e3a1c75fc471d1c9dcc638139b35017990b93b0 Mon Sep 17 00:00:00 2001 From: Carolyn MacLeod Date: Tue, 20 Mar 2018 23:05:04 -0400 Subject: [PATCH 5/8] Remove focus outline, lighten menubar items' hover background, remove hover css for separator, fix comment. --- examples/menubar/menubar-2/css/menubarAction.css | 14 +++++++------- examples/menubar/menubar-2/js/PopupMenuAction.js | 3 +-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/examples/menubar/menubar-2/css/menubarAction.css b/examples/menubar/menubar-2/css/menubarAction.css index a26fbc334e..0d64977c78 100644 --- a/examples/menubar/menubar-2/css/menubarAction.css +++ b/examples/menubar/menubar-2/css/menubarAction.css @@ -68,18 +68,18 @@ ul[role="menubar"] [role="separator"] { background-repeat: repeat-x; } +ul[role="menubar"]:not(.focus) [role="menuitem"]:hover { + background-color: #444444; + color: white; +} + ul[role="menubar"] [role="menuitem"]:focus, -ul[role="menubar"]:not(.focus) [role="menuitem"]:hover, ul[role="menubar"] [role="menu"] [role="menuitem"]:focus, -ul[role="menubar"] [role="menu"] [role="menuitem"]:hover, ul[role="menubar"] [role="menu"] [role="menuitemcheckbox"]:focus, -ul[role="menubar"] [role="menu"] [role="menuitemcheckbox"]:hover, -ul[role="menubar"] [role="menu"] [role="menuitemradio"]:focus, -ul[role="menubar"] [role="menu"] [role="menuitemradio"]:hover, -ul[role="menubar"] [role="menu"] [role="separator"]:focus, -ul[role="menubar"] [role="menu"] [role="separator"]:hover { +ul[role="menubar"] [role="menu"] [role="menuitemradio"]:focus { background-color: black; color: white; + outline: none; } ul[role="menubar"] [role="menuitemcheckbox"][aria-checked='true'], diff --git a/examples/menubar/menubar-2/js/PopupMenuAction.js b/examples/menubar/menubar-2/js/PopupMenuAction.js index b8824dadde..ec445dee05 100644 --- a/examples/menubar/menubar-2/js/PopupMenuAction.js +++ b/examples/menubar/menubar-2/js/PopupMenuAction.js @@ -56,8 +56,7 @@ var PopupMenuAction = function (domNode, controllerObj, actionManager) { * @method PopupMenuAction.prototype.init * * @desc -* Add domNode event listeners for mouseover and mouseout. Traverse -* domNode children to configure each menuitem and populate menuitems +* Traverse domNode children to configure each menuitem and populate menuitems * array. Initialize firstItem and lastItem properties. */ PopupMenuAction.prototype.init = function () { From fde9f5e62bec50c930247e7dc6ff166b732e2361 Mon Sep 17 00:00:00 2001 From: Carolyn MacLeod Date: Tue, 20 Mar 2018 23:29:38 -0400 Subject: [PATCH 6/8] Keep focus style on menubar item when menu is open. --- examples/menubar/menubar-2/css/menubarAction.css | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/menubar/menubar-2/css/menubarAction.css b/examples/menubar/menubar-2/css/menubarAction.css index 0d64977c78..ddec804817 100644 --- a/examples/menubar/menubar-2/css/menubarAction.css +++ b/examples/menubar/menubar-2/css/menubarAction.css @@ -74,6 +74,7 @@ ul[role="menubar"]:not(.focus) [role="menuitem"]:hover { } ul[role="menubar"] [role="menuitem"]:focus, +ul[role="menubar"] [role="menuitem"][aria-expanded="true"], ul[role="menubar"] [role="menu"] [role="menuitem"]:focus, ul[role="menubar"] [role="menu"] [role="menuitemcheckbox"]:focus, ul[role="menubar"] [role="menu"] [role="menuitemradio"]:focus { From 153d651a9916b59058f94d4e72d8874339cde510 Mon Sep 17 00:00:00 2001 From: Carolyn MacLeod Date: Tue, 20 Mar 2018 23:43:16 -0400 Subject: [PATCH 7/8] Make travis happy by adding newline at eof. --- examples/menubar/menubar-2/js/PopupMenuItemAction.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/menubar/menubar-2/js/PopupMenuItemAction.js b/examples/menubar/menubar-2/js/PopupMenuItemAction.js index 343086a36f..a592890c18 100644 --- a/examples/menubar/menubar-2/js/PopupMenuItemAction.js +++ b/examples/menubar/menubar-2/js/PopupMenuItemAction.js @@ -203,4 +203,4 @@ MenuItem.prototype.handleClick = function (event) { MenuItem.prototype.handleMouseover = function (event) { this.menu.setFocusToItem(this); -}; \ No newline at end of file +}; From 00ca9dc25855ae37f7a90e72c7fc73eb32d59921 Mon Sep 17 00:00:00 2001 From: Carolyn MacLeod Date: Wed, 21 Mar 2018 08:49:37 -0400 Subject: [PATCH 8/8] Separate menubar item focus style from menuitem focus styles. Add border to focused menubar item. --- examples/menubar/menubar-2/css/menubarAction.css | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/examples/menubar/menubar-2/css/menubarAction.css b/examples/menubar/menubar-2/css/menubarAction.css index ddec804817..529fa796c9 100644 --- a/examples/menubar/menubar-2/css/menubarAction.css +++ b/examples/menubar/menubar-2/css/menubarAction.css @@ -74,7 +74,13 @@ ul[role="menubar"]:not(.focus) [role="menuitem"]:hover { } ul[role="menubar"] [role="menuitem"]:focus, -ul[role="menubar"] [role="menuitem"][aria-expanded="true"], +ul[role="menubar"] [role="menuitem"][aria-expanded="true"] { + background-color: black; + color: white; + border: 2px solid black; + outline: none; +} + ul[role="menubar"] [role="menu"] [role="menuitem"]:focus, ul[role="menubar"] [role="menu"] [role="menuitemcheckbox"]:focus, ul[role="menubar"] [role="menu"] [role="menuitemradio"]:focus {