From 35461fb2363eec7bf238b3c36b521bf5f175f659 Mon Sep 17 00:00:00 2001 From: Katie McFaul Date: Mon, 2 May 2022 11:21:46 -0400 Subject: [PATCH 1/3] fix: tree view menu keyboard handling --- packages/react-core/src/components/Menu/Menu.tsx | 6 +++++- .../examples/ComposableTreeViewMenu.tsx | 13 +++++++++---- packages/react-core/src/helpers/KeyboardHandler.tsx | 9 +++++++-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/react-core/src/components/Menu/Menu.tsx b/packages/react-core/src/components/Menu/Menu.tsx index b9e6bac329b..3a4b2e0107a 100644 --- a/packages/react-core/src/components/Menu/Menu.tsx +++ b/packages/react-core/src/components/Menu/Menu.tsx @@ -59,6 +59,8 @@ export interface MenuProps extends Omit, 'r isPlain?: boolean; /** Indicates if the menu should be srollable */ isScrollable?: boolean; + /** Props spread to keyboard handler component for custom menu structures */ + customKeyboardHandler?: any; } export interface MenuState { @@ -209,7 +211,7 @@ class MenuBase extends React.Component { ? Array.from(this.activeMenu.getElementsByTagName('UL')[0].children).filter( el => !(el.classList.contains('pf-m-disabled') || el.classList.contains('pf-c-divider')) ) - : Array.from(this.activeMenu.getElementsByTagName('LI')).filter( + : Array.from(this.menuRef.current.getElementsByTagName('LI')).filter( el => !(el.classList.contains('pf-m-disabled') || el.classList.contains('pf-c-divider')) ); }; @@ -238,6 +240,7 @@ class MenuBase extends React.Component { onGetMenuHeight, parentMenu = null, activeItemId = null, + customKeyboardHandler, /* eslint-disable @typescript-eslint/no-unused-vars */ innerRef, isRootMenu, @@ -282,6 +285,7 @@ class MenuBase extends React.Component { } noEnterHandling noSpaceHandling + {...customKeyboardHandler} /> )}
{ return; } if (menuRef.current.contains(event.target as Node) || toggleRef.current.contains(event.target as Node)) { - if (event.key === 'Escape' || event.key === 'Tab') { + if (event.key === 'Escape') { setIsOpen(!isOpen); toggleRef.current.focus(); } @@ -194,7 +194,7 @@ export const ComposableTreeViewMenu: React.FunctionComponent = () => { ev.stopPropagation(); // Stop handleClickOutside from handling setTimeout(() => { if (menuRef.current) { - const firstElement = menuRef.current.querySelector('li > button:not(:disabled)'); + const firstElement = menuRef.current.querySelector('li button:not(:disabled)'); firstElement && (firstElement as HTMLElement).focus(); } }, 0); @@ -211,13 +211,18 @@ export const ComposableTreeViewMenu: React.FunctionComponent = () => { const menu = ( console.log('selected', itemId)} style={ { '--pf-c-menu--Width': '300px' } as React.CSSProperties } + customKeyboardHandler={{ + isActiveElement: (element: Element) => document.activeElement.closest('li') === element, + getFocusableElement: (navigableElement: Element) => + navigableElement.querySelectorAll('button, input')[0] as Element, + validSiblingTags: ['button', 'input'], + onlyTraverseSiblings: false + }} > diff --git a/packages/react-core/src/helpers/KeyboardHandler.tsx b/packages/react-core/src/helpers/KeyboardHandler.tsx index 4c386aad7ee..530ca07bcfe 100644 --- a/packages/react-core/src/helpers/KeyboardHandler.tsx +++ b/packages/react-core/src/helpers/KeyboardHandler.tsx @@ -18,6 +18,8 @@ export interface KeyboardHandlerProps { validSiblingTags?: string[]; /** Flag indicating that the tabIndex of the currently focused element and next focused element should be updated, in the case of using a roving tabIndex */ updateTabIndex?: boolean; + /** Flag indicating that next focusable element of a horizontal movement will be this element's sibling */ + onlyTraverseSiblings?: boolean; /** Flag indicating that the included vertical arrow key handling should be ignored */ noVerticalArrowHandling?: boolean; /** Flag indicating that the included horizontal arrow key handling should be ignored */ @@ -176,6 +178,7 @@ export class KeyboardHandler extends React.Component { isActiveElement: (navigableElement: Element) => document.activeElement === navigableElement, getFocusableElement: (navigableElement: Element) => navigableElement, validSiblingTags: ['BUTTON', 'A'], + onlyTraverseSiblings: true, updateTabIndex: true, noHorizontalArrowHandling: false, noVerticalArrowHandling: false, @@ -212,7 +215,8 @@ export class KeyboardHandler extends React.Component { updateTabIndex, validSiblingTags, additionalKeyHandler, - createNavigableElements + createNavigableElements, + onlyTraverseSiblings } = this.props; // Pass the event off to be handled by any custom handler @@ -256,7 +260,8 @@ export class KeyboardHandler extends React.Component { validSiblingTags, noVerticalArrowHandling, noHorizontalArrowHandling, - updateTabIndex + updateTabIndex, + onlyTraverseSiblings ); }; From ded5d4d3ed4ee90fb14a4dc0062244a088941b58 Mon Sep 17 00:00:00 2001 From: Katie McFaul Date: Tue, 3 May 2022 11:52:26 -0400 Subject: [PATCH 2/3] refactor to panel --- .../react-core/src/components/Panel/Panel.tsx | 9 +- .../demos/ComposableMenu/ComposableMenu.md | 2 + .../examples/ComposableTreeViewMenu.tsx | 82 ++++++++++++------- 3 files changed, 63 insertions(+), 30 deletions(-) diff --git a/packages/react-core/src/components/Panel/Panel.tsx b/packages/react-core/src/components/Panel/Panel.tsx index a17f80b015f..77d7e0df4c8 100644 --- a/packages/react-core/src/components/Panel/Panel.tsx +++ b/packages/react-core/src/components/Panel/Panel.tsx @@ -11,13 +11,16 @@ export interface PanelProps extends React.HTMLProps { variant?: 'raised' | 'bordered'; /** Flag to add scrollable styling to the panel */ isScrollable?: boolean; + /** @hide Forwarded ref */ + innerRef?: React.Ref; } -export const Panel: React.FunctionComponent = ({ +const PanelBase: React.FunctionComponent = ({ className, children, variant, isScrollable, + innerRef, ...props }: PanelProps) => (
= ({ isScrollable && styles.modifiers.scrollable, className )} + ref={innerRef} {...props} > {children}
); +export const Panel = React.forwardRef((props: PanelProps, ref: React.Ref) => ( + +)); Panel.displayName = 'Panel'; diff --git a/packages/react-core/src/demos/ComposableMenu/ComposableMenu.md b/packages/react-core/src/demos/ComposableMenu/ComposableMenu.md index cea9048e950..eb410d2f7b6 100644 --- a/packages/react-core/src/demos/ComposableMenu/ComposableMenu.md +++ b/packages/react-core/src/demos/ComposableMenu/ComposableMenu.md @@ -47,6 +47,8 @@ Composable menus currently require consumer keyboard handling and use of our und ### Composable tree view menu +When rendering a menu-like element that does not contain MenuItem components, [Panel](/components/panel) allows more flexible control and customization. + ```ts file="./examples/ComposableTreeViewMenu.tsx" ``` diff --git a/packages/react-core/src/demos/ComposableMenu/examples/ComposableTreeViewMenu.tsx b/packages/react-core/src/demos/ComposableMenu/examples/ComposableTreeViewMenu.tsx index 564de654336..6a503cef55f 100644 --- a/packages/react-core/src/demos/ComposableMenu/examples/ComposableTreeViewMenu.tsx +++ b/packages/react-core/src/demos/ComposableMenu/examples/ComposableTreeViewMenu.tsx @@ -1,10 +1,10 @@ import React from 'react'; import { MenuToggle, - Menu, - MenuContent, - MenuGroup, - MenuList, + Panel, + PanelMain, + PanelMainBody, + Title, Popper, TreeView, TreeViewDataItem @@ -14,6 +14,7 @@ export const ComposableTreeViewMenu: React.FunctionComponent = () => { const [isOpen, setIsOpen] = React.useState(false); const [checkedItems, setCheckedItems] = React.useState([]); const toggleRef = React.useRef(); + const containerRef = React.useRef(); const menuRef = React.useRef(); const statusOptions: TreeViewDataItem[] = [ @@ -172,6 +173,15 @@ export const ComposableTreeViewMenu: React.FunctionComponent = () => { setIsOpen(!isOpen); toggleRef.current.focus(); } + + if (event.key === 'Tab' && !event.shiftKey) { + const treeList = menuRef.current.querySelectorAll('.pf-c-tree-view'); + if (treeList[treeList.length - 1].contains(event.target as Node)) { + event.preventDefault(); + setIsOpen(!isOpen); + toggleRef.current.focus(); + } + } } }; @@ -202,44 +212,58 @@ export const ComposableTreeViewMenu: React.FunctionComponent = () => { }; const toggle = ( - - {isOpen ? 'Expanded' : 'Collapsed'} - +
+ + {isOpen ? 'Expanded' : 'Collapsed'} + +
); const statusMapped = statusOptions.map(mapTree); const roleMapped = roleOptions.map(mapTree); const menu = ( - document.activeElement.closest('li') === element, - getFocusableElement: (navigableElement: Element) => - navigableElement.querySelectorAll('button, input')[0] as Element, - validSiblingTags: ['button', 'input'], - onlyTraverseSiblings: false + variant="raised" + style={{ + width: '300px' }} > - - - + +
+ + + Status + + + onCheck(event, item, 'status')} /> - - + +
+
+ + + Roles + + + onCheck(event, item, 'role')} /> - - - -
+ + + + + ); + return ( + ); - return ; }; From 505a76a1585c7f5a7c36a8deb981aa136977fd95 Mon Sep 17 00:00:00 2001 From: Katie McFaul Date: Wed, 4 May 2022 10:52:19 -0400 Subject: [PATCH 3/3] add comments to helper functions, remove unused keyboard override from menu --- packages/react-core/src/components/Menu/Menu.tsx | 4 ---- .../ComposableMenu/examples/ComposableTreeViewMenu.tsx | 7 ++++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/react-core/src/components/Menu/Menu.tsx b/packages/react-core/src/components/Menu/Menu.tsx index 3a4b2e0107a..31020ec356d 100644 --- a/packages/react-core/src/components/Menu/Menu.tsx +++ b/packages/react-core/src/components/Menu/Menu.tsx @@ -59,8 +59,6 @@ export interface MenuProps extends Omit, 'r isPlain?: boolean; /** Indicates if the menu should be srollable */ isScrollable?: boolean; - /** Props spread to keyboard handler component for custom menu structures */ - customKeyboardHandler?: any; } export interface MenuState { @@ -240,7 +238,6 @@ class MenuBase extends React.Component { onGetMenuHeight, parentMenu = null, activeItemId = null, - customKeyboardHandler, /* eslint-disable @typescript-eslint/no-unused-vars */ innerRef, isRootMenu, @@ -285,7 +282,6 @@ class MenuBase extends React.Component { } noEnterHandling noSpaceHandling - {...customKeyboardHandler} /> )}
{ customBadgeContent: 0 } ]; - // Helper functions + // Helper functions for tree const isChecked = (dataItem: TreeViewDataItem) => checkedItems.some(item => item.id === dataItem.id); const areAllDescendantsChecked = (dataItem: TreeViewDataItem) => dataItem.children ? dataItem.children.every(child => areAllDescendantsChecked(child)) : isChecked(dataItem); @@ -164,16 +164,20 @@ export const ComposableTreeViewMenu: React.FunctionComponent = () => { ); }; + // Controls keys that should open/close the menu const handleMenuKeys = (event: KeyboardEvent) => { if (!isOpen) { return; } if (menuRef.current.contains(event.target as Node) || toggleRef.current.contains(event.target as Node)) { + // The escape key when pressed while inside the menu should close the menu and refocus the toggle if (event.key === 'Escape') { setIsOpen(!isOpen); toggleRef.current.focus(); } + // The tab key when pressed while inside the menu and on the contained last tree view should close the menu and refocus the toggle + // Shift tab should keep the default behavior to return to a previous tree view if (event.key === 'Tab' && !event.shiftKey) { const treeList = menuRef.current.querySelectorAll('.pf-c-tree-view'); if (treeList[treeList.length - 1].contains(event.target as Node)) { @@ -185,6 +189,7 @@ export const ComposableTreeViewMenu: React.FunctionComponent = () => { } }; + // Controls that a click outside the menu while the menu is open should close the menu const handleClickOutside = (event: MouseEvent) => { if (isOpen && !menuRef.current.contains(event.target as Node)) { setIsOpen(false);