From 016459486eac17d3c6f3fab48fad9bb21edce08f Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Wed, 15 Jan 2025 16:02:09 -0800 Subject: [PATCH 01/80] feat: added context values for keyboard navigation --- client/components/Menubar/contexts.jsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/components/Menubar/contexts.jsx b/client/components/Menubar/contexts.jsx index ab3bb9ffc..f97c3ea8f 100644 --- a/client/components/Menubar/contexts.jsx +++ b/client/components/Menubar/contexts.jsx @@ -1,3 +1,4 @@ +import { set } from 'lodash'; import { createContext } from 'react'; export const ParentMenuContext = createContext('none'); @@ -7,5 +8,9 @@ export const MenuOpenContext = createContext('none'); export const MenubarContext = createContext({ createMenuHandlers: () => ({}), createMenuItemHandlers: () => ({}), - toggleMenuOpen: () => {} + toggleMenuOpen: () => {}, + activeIndex: -1, + setActiveIndex: () => {}, + registerItem: () => {}, + menuItems: [] }); From 656b7405d28424440c6615d28aad0b292b703a29 Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Thu, 16 Jan 2025 00:10:41 -0800 Subject: [PATCH 02/80] feat: top-level keyboard navigation added --- client/components/Menubar/Menubar.jsx | 103 ++++++++++++++++--- client/components/Menubar/MenubarItem.jsx | 8 +- client/components/Menubar/MenubarSubmenu.jsx | 67 +++++++----- 3 files changed, 140 insertions(+), 38 deletions(-) diff --git a/client/components/Menubar/Menubar.jsx b/client/components/Menubar/Menubar.jsx index b80624651..21d10fbe4 100644 --- a/client/components/Menubar/Menubar.jsx +++ b/client/components/Menubar/Menubar.jsx @@ -2,18 +2,87 @@ import PropTypes from 'prop-types'; import React, { useCallback, useMemo, useRef, useState } from 'react'; import useModalClose from '../../common/useModalClose'; import { MenuOpenContext, MenubarContext } from './contexts'; +import useKeyDownHandlers from '../../common/useKeyDownHandlers'; function Menubar({ children, className }) { const [menuOpen, setMenuOpen] = useState('none'); - + const [activeIndex, setActiveIndex] = useState(-1); + const [menuItems, setMenuItems] = useState([]); const timerRef = useRef(null); + const nodeRef = useRef(null); + + const registerItem = useCallback((id) => { + setMenuItems((prev) => [...prev, id]); + return () => { + setMenuItems((prev) => prev.filter((item) => item !== id)); + }; + }, []); + + const toggleMenuOpen = useCallback( + (menu) => { + setMenuOpen((prevState) => (prevState === menu ? 'none' : menu)); + }, + [setMenuOpen] + ); + + const keyHandlers = useMemo( + () => ({ + ArrowUp: (e) => { + e.preventDefault(); + // if submenu is closed, open it and focus the last item + // if submenu is open, focus the previous item + }, + ArrowDown: (e) => { + e.preventDefault(); + + // if submenu is closed, open it and focus the first item + // if submenu is open, focus the next item + }, + ArrowLeft: (e) => { + e.preventDefault(); + console.log('left'); + setActiveIndex( + (prev) => (prev - 1 + menuItems.length) % menuItems.length + ); + // if submenu is open, close it, open the next one and focus the next top-level item + }, + ArrowRight: (e) => { + e.preventDefault(); + console.log('right'); + setActiveIndex((prev) => (prev + 1) % menuItems.length); + // if submenu is open, close it, open previous one and focus the previous top-level item + }, + Enter: (e) => { + e.preventDefault(); + // if submenu is open, activate the focused item + // if submenu is closed, open it and focus the first item + toggleMenuOpen(menuItems[activeIndex]); + }, + ' ': (e) => { + // same as Enter + e.preventDefault(); + // if submenu is open, activate the focused item + // if submenu is closed, open it and focus the first item + toggleMenuOpen(menuItems[activeIndex]); + }, + Escape: (e) => { + // close all submenus + setMenuOpen('none'); + }, + Tab: (e) => { + // close + } + // support direct access keys + }), + [menuItems, menuOpen, activeIndex, toggleMenuOpen] + ); + + useKeyDownHandlers(keyHandlers); const handleClose = useCallback(() => { setMenuOpen('none'); }, [setMenuOpen]); - const nodeRef = useModalClose(handleClose); - const clearHideTimeout = useCallback(() => { if (timerRef.current) { clearTimeout(timerRef.current); @@ -22,16 +91,12 @@ function Menubar({ children, className }) { }, [timerRef]); const handleBlur = useCallback(() => { - timerRef.current = setTimeout(() => setMenuOpen('none'), 10); + timerRef.current = setTimeout(() => { + setMenuOpen('none'); + setActiveIndex(-1); + }, 10); }, [timerRef, setMenuOpen]); - const toggleMenuOpen = useCallback( - (menu) => { - setMenuOpen((prevState) => (prevState === menu ? 'none' : menu)); - }, - [setMenuOpen] - ); - const contextValue = useMemo( () => ({ createMenuHandlers: (menu) => ({ @@ -57,9 +122,21 @@ function Menubar({ children, className }) { setMenuOpen(menu); } }), - toggleMenuOpen + toggleMenuOpen, + activeIndex, + setActiveIndex, + registerItem, + menuItems }), - [setMenuOpen, toggleMenuOpen, clearHideTimeout, handleBlur] + [ + menuOpen, + toggleMenuOpen, + clearHideTimeout, + handleBlur, + activeIndex, + registerItem, + menuItems + ] ); return ( diff --git a/client/components/Menubar/MenubarItem.jsx b/client/components/Menubar/MenubarItem.jsx index 8d595bb5c..dcfebe60b 100644 --- a/client/components/Menubar/MenubarItem.jsx +++ b/client/components/Menubar/MenubarItem.jsx @@ -28,7 +28,13 @@ function MenubarItem({ return (
  • - +
    • ); } diff --git a/client/components/Menubar/MenubarSubmenu.jsx b/client/components/Menubar/MenubarSubmenu.jsx index 13b0e3317..ef917d0ac 100644 --- a/client/components/Menubar/MenubarSubmenu.jsx +++ b/client/components/Menubar/MenubarSubmenu.jsx @@ -2,7 +2,7 @@ import classNames from 'classnames'; import PropTypes from 'prop-types'; -import React, { useContext, useMemo } from 'react'; +import React, { useEffect, useContext, useRef, useMemo } from 'react'; import TriangleIcon from '../../images/down-filled-triangle.svg'; import { MenuOpenContext, MenubarContext, ParentMenuContext } from './contexts'; @@ -24,27 +24,29 @@ export function useMenuProps(id) { /* ------------------------------------------------------------------------------------------------- * MenubarTrigger * -----------------------------------------------------------------------------------------------*/ - -function MenubarTrigger({ id, title, role, hasPopup, ...props }) { - const { isOpen, handlers } = useMenuProps(id); - - return ( - - ); -} +const MenubarTrigger = React.forwardRef( + ({ id, title, role, hasPopup, ...props }, ref) => { + const { isOpen, handlers } = useMenuProps(id); + + return ( + + ); + } +); MenubarTrigger.propTypes = { id: PropTypes.string.isRequired, @@ -95,20 +97,37 @@ function MenubarSubmenu({ listRole: customListRole, ...props }) { - const { isOpen } = useMenuProps(id); + const { isOpen, handlers } = useMenuProps(id); + const { activeIndex, menuItems, registerItem } = useContext(MenubarContext); + const isActive = menuItems[activeIndex] === id; + const buttonRef = useRef(null); const triggerRole = customTriggerRole || 'menuitem'; const listRole = customListRole || 'menu'; - const hasPopup = listRole === 'listbox' ? 'listbox' : 'menu'; + useEffect(() => { + if (isActive && buttonRef.current) { + buttonRef.current.focus(); + } + }, [isActive]); + + // register this menu item + useEffect(() => { + const unregister = registerItem(id); + return unregister; + }, [id, registerItem]); + return (
  • From ec66297e6aa0466d59ab15d8a0d4808e07fa9486 Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Thu, 16 Jan 2025 16:04:26 -0800 Subject: [PATCH 03/80] feat: implemented ref forwarding --- client/common/ButtonOrLink.jsx | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/client/common/ButtonOrLink.jsx b/client/common/ButtonOrLink.jsx index 924f10802..fa7258dff 100644 --- a/client/common/ButtonOrLink.jsx +++ b/client/common/ButtonOrLink.jsx @@ -5,23 +5,34 @@ import PropTypes from 'prop-types'; /** * Helper for switching between ; -}; + return ( + + ); +}); /** * Accepts all the props of an HTML or
    • - {t('Nav.File.New')} + + {t('Nav.File.New')} + { {metaKeyName}+S dispatch(cloneProject())} > {t('Nav.File.Duplicate')} - + {t('Nav.File.Share')} - + {t('Nav.File.Download')} {t('Nav.File.Open')} { {t('Nav.File.AddToCollection')} @@ -210,32 +221,38 @@ const ProjectMenu = () => { - + {t('Nav.Edit.TidyCode')} {metaKeyName}+Shift+F - + {t('Nav.Edit.Find')} {metaKeyName}+F - + {t('Nav.Edit.Replace')} {replaceCommand} - dispatch(newFile(rootFile.id))}> + dispatch(newFile(rootFile.id))} + > {t('Nav.Sketch.AddFile')} {newFileCommand} - dispatch(newFolder(rootFile.id))}> + dispatch(newFolder(rootFile.id))} + > {t('Nav.Sketch.AddFolder')} - dispatch(startSketch())}> + dispatch(startSketch())}> {t('Nav.Sketch.Run')} {metaKeyName}+Enter - dispatch(stopSketch())}> + dispatch(stopSketch())}> {t('Nav.Sketch.Stop')} Shift+{metaKeyName}+Enter @@ -243,13 +260,18 @@ const ProjectMenu = () => { - dispatch(showKeyboardShortcutModal())}> + dispatch(showKeyboardShortcutModal())} + > {t('Nav.Help.KeyboardShortcuts')} - + {t('Nav.Help.Reference')} - {t('Nav.Help.About')} + + {t('Nav.Help.About')} + {getConfig('TRANSLATIONS_ENABLED') && } @@ -275,6 +297,7 @@ const LanguageMenu = () => { {sortBy(availableLanguages).map((key) => ( // eslint-disable-next-line react/jsx-no-bind { } > - + {t('Nav.Auth.MySketches')} {t('Nav.Auth.MyCollections')} - + {t('Nav.Auth.MyAssets')} - {t('Preferences.Settings')} - dispatch(logoutUser())}> + + {t('Preferences.Settings')} + + dispatch(logoutUser())}> {t('Nav.Auth.LogOut')} From 49e3ddb650681d4b0adb4c7858d13d522f42a2cb Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Thu, 16 Jan 2025 16:11:14 -0800 Subject: [PATCH 06/80] feat: updated with relevant submenu states and key handlers --- client/components/Menubar/Menubar.jsx | 56 ++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/client/components/Menubar/Menubar.jsx b/client/components/Menubar/Menubar.jsx index 21d10fbe4..3ae2bc0b0 100644 --- a/client/components/Menubar/Menubar.jsx +++ b/client/components/Menubar/Menubar.jsx @@ -8,6 +8,8 @@ function Menubar({ children, className }) { const [menuOpen, setMenuOpen] = useState('none'); const [activeIndex, setActiveIndex] = useState(-1); const [menuItems, setMenuItems] = useState([]); + const [submenuActiveIndex, setSubmenuActiveIndex] = useState(-1); + const [submenuItems, setSubmenuItems] = useState([]); const timerRef = useRef(null); const nodeRef = useRef(null); @@ -18,6 +20,13 @@ function Menubar({ children, className }) { }; }, []); + const registerSubmenuItem = useCallback((id) => { + setSubmenuItems((prev) => [...prev, id]); + return () => { + setSubmenuItems((prev) => prev.filter((item) => item !== id)); + }; + }, []); + const toggleMenuOpen = useCallback( (menu) => { setMenuOpen((prevState) => (prevState === menu ? 'none' : menu)); @@ -30,27 +39,49 @@ function Menubar({ children, className }) { ArrowUp: (e) => { e.preventDefault(); // if submenu is closed, open it and focus the last item + if (menuOpen === 'none') { + toggleMenuOpen(menuItems[activeIndex]); + setSubmenuActiveIndex(submenuItems.length - 1); // focus last + } else { + setSubmenuActiveIndex( + (prev) => (prev - 1 + submenuItems.length) % submenuItems.length + ); + } // if submenu is open, focus the previous item }, ArrowDown: (e) => { e.preventDefault(); - // if submenu is closed, open it and focus the first item + if (menuOpen === 'none') { + toggleMenuOpen(menuItems[activeIndex]); + setSubmenuActiveIndex(0); // focus first + } else { + setSubmenuActiveIndex((prev) => (prev + 1) % submenuItems.length); + } // if submenu is open, focus the next item }, ArrowLeft: (e) => { e.preventDefault(); - console.log('left'); - setActiveIndex( - (prev) => (prev - 1 + menuItems.length) % menuItems.length - ); + const newIndex = + (activeIndex - 1 + menuItems.length) % menuItems.length; + setActiveIndex(newIndex); + // if submenu is open, close it, open the next one and focus the next top-level item + if (menuOpen !== 'none') { + toggleMenuOpen(menuItems[activeIndex]); + setMenuOpen(menuItems[newIndex]); + } }, ArrowRight: (e) => { e.preventDefault(); - console.log('right'); - setActiveIndex((prev) => (prev + 1) % menuItems.length); + const newIndex = (activeIndex + 1) % menuItems.length; + setActiveIndex(newIndex); + // if submenu is open, close it, open previous one and focus the previous top-level item + if (menuOpen !== 'none') { + toggleMenuOpen(menuItems[activeIndex]); + setMenuOpen(menuItems[newIndex]); + } }, Enter: (e) => { e.preventDefault(); @@ -126,7 +157,11 @@ function Menubar({ children, className }) { activeIndex, setActiveIndex, registerItem, - menuItems + menuItems, + submenuActiveIndex, + setSubmenuActiveIndex, + registerSubmenuItem, + submenuItems }), [ menuOpen, @@ -135,7 +170,10 @@ function Menubar({ children, className }) { handleBlur, activeIndex, registerItem, - menuItems + menuItems, + submenuActiveIndex, + registerSubmenuItem, + submenuItems ] ); From a300c5205040d8b27fc832e633fd3f79486720a9 Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Thu, 16 Jan 2025 16:18:52 -0800 Subject: [PATCH 07/80] feat: menubaritem registers self and determines if active or not --- client/components/Menubar/MenubarItem.jsx | 40 +++++++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/client/components/Menubar/MenubarItem.jsx b/client/components/Menubar/MenubarItem.jsx index dcfebe60b..51aeea69f 100644 --- a/client/components/Menubar/MenubarItem.jsx +++ b/client/components/Menubar/MenubarItem.jsx @@ -1,18 +1,25 @@ import PropTypes from 'prop-types'; -import React, { useContext, useMemo } from 'react'; +import React, { useEffect, useContext, useRef, useMemo } from 'react'; import ButtonOrLink from '../../common/ButtonOrLink'; import { MenubarContext, ParentMenuContext } from './contexts'; function MenubarItem({ + id, hideIf, className, role: customRole, selected, ...rest }) { + const submenuItemRef = useRef(null); const parent = useContext(ParentMenuContext); - const { createMenuItemHandlers } = useContext(MenubarContext); + const { + createMenuItemHandlers, + registerSubmenuItem, + submenuActiveIndex, + submenuItems + } = useContext(MenubarContext); const handlers = useMemo(() => createMenuItemHandlers(parent), [ createMenuItemHandlers, @@ -25,15 +32,41 @@ function MenubarItem({ const role = customRole || 'menuitem'; const ariaSelected = role === 'option' ? { 'aria-selected': selected } : {}; + const isActive = submenuItems[submenuActiveIndex] === id; + + useEffect(() => { + if (isActive && submenuItemRef.current) { + submenuItemRef.current.focus(); + } + }, [isActive, submenuActiveIndex]); + + useEffect(() => { + const unregister = registerSubmenuItem(id); + return unregister; + }, [id, registerSubmenuItem]); + + useEffect(() => { + if (isActive) { + console.log('MenubarItem Focus:', { + id, + isActive, + parent, + submenuActiveIndex, + element: submenuItemRef.current + }); + } + }, [isActive, id, parent, submenuActiveIndex]); return (
    • ); @@ -41,6 +74,7 @@ function MenubarItem({ MenubarItem.propTypes = { ...ButtonOrLink.propTypes, + id: PropTypes.string.isRequired, onClick: PropTypes.func, value: PropTypes.string, /** From 93f963d8740d14a6f287b128c7d940287ee2b184 Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Thu, 16 Jan 2025 23:19:56 -0800 Subject: [PATCH 08/80] feat: base implementation of keyboard-navigable submenus --- client/components/Menubar/MenubarItem.jsx | 16 ++- client/components/Menubar/MenubarSubmenu.jsx | 103 ++++++++++++++++++- client/components/Menubar/contexts.jsx | 4 +- 3 files changed, 108 insertions(+), 15 deletions(-) diff --git a/client/components/Menubar/MenubarItem.jsx b/client/components/Menubar/MenubarItem.jsx index 51aeea69f..6f0d85b75 100644 --- a/client/components/Menubar/MenubarItem.jsx +++ b/client/components/Menubar/MenubarItem.jsx @@ -1,7 +1,7 @@ import PropTypes from 'prop-types'; import React, { useEffect, useContext, useRef, useMemo } from 'react'; import ButtonOrLink from '../../common/ButtonOrLink'; -import { MenubarContext, ParentMenuContext } from './contexts'; +import { MenubarContext, SubmenuContext, ParentMenuContext } from './contexts'; function MenubarItem({ id, @@ -13,13 +13,11 @@ function MenubarItem({ }) { const submenuItemRef = useRef(null); const parent = useContext(ParentMenuContext); + const { createMenuItemHandlers } = useContext(MenubarContext); - const { - createMenuItemHandlers, - registerSubmenuItem, - submenuActiveIndex, - submenuItems - } = useContext(MenubarContext); + const { registerSubmenuItem, submenuActiveIndex, submenuItems } = useContext( + SubmenuContext + ); const handlers = useMemo(() => createMenuItemHandlers(parent), [ createMenuItemHandlers, @@ -32,13 +30,13 @@ function MenubarItem({ const role = customRole || 'menuitem'; const ariaSelected = role === 'option' ? { 'aria-selected': selected } : {}; - const isActive = submenuItems[submenuActiveIndex] === id; + const isActive = submenuItems[submenuActiveIndex] === id; // is this item active in its own submenu? useEffect(() => { if (isActive && submenuItemRef.current) { submenuItemRef.current.focus(); } - }, [isActive, submenuActiveIndex]); + }, [isActive, submenuItemRef]); useEffect(() => { const unregister = registerSubmenuItem(id); diff --git a/client/components/Menubar/MenubarSubmenu.jsx b/client/components/Menubar/MenubarSubmenu.jsx index ef917d0ac..a12210c42 100644 --- a/client/components/Menubar/MenubarSubmenu.jsx +++ b/client/components/Menubar/MenubarSubmenu.jsx @@ -2,9 +2,22 @@ import classNames from 'classnames'; import PropTypes from 'prop-types'; -import React, { useEffect, useContext, useRef, useMemo } from 'react'; +import React, { + useState, + useEffect, + useCallback, + useContext, + useRef, + useMemo +} from 'react'; import TriangleIcon from '../../images/down-filled-triangle.svg'; -import { MenuOpenContext, MenubarContext, ParentMenuContext } from './contexts'; +import { + MenuOpenContext, + MenubarContext, + SubmenuContext, + ParentMenuContext +} from './contexts'; +import useKeyDownHandlers from '../../common/useKeyDownHandlers'; export function useMenuProps(id) { const activeMenu = useContext(MenuOpenContext); @@ -99,6 +112,8 @@ function MenubarSubmenu({ }) { const { isOpen, handlers } = useMenuProps(id); const { activeIndex, menuItems, registerItem } = useContext(MenubarContext); + const [submenuItems, setSubmenuItems] = useState([]); + const [submenuActiveIndex, setSubmenuActiveIndex] = useState(-1); const isActive = menuItems[activeIndex] === id; const buttonRef = useRef(null); @@ -106,6 +121,55 @@ function MenubarSubmenu({ const listRole = customListRole || 'menu'; const hasPopup = listRole === 'listbox' ? 'listbox' : 'menu'; + const keyHandlers = useMemo(() => { + // we only want to create the handlers if the menu is open, + // otherwise return empty handlers + if (!isOpen) { + return {}; + } + + return { + ArrowUp: (e) => { + e.preventDefault(); + e.stopPropagation(); + + setSubmenuActiveIndex((prev) => { + const newIndex = + (prev - 1 + submenuItems.length) % submenuItems.length; + return newIndex; + }); + }, + ArrowDown: (e) => { + e.preventDefault(); + e.stopPropagation(); + + setSubmenuActiveIndex((prev) => { + const newIndex = (prev + 1) % submenuItems.length; + return newIndex; + }); + }, + Enter: (e) => { + e.preventDefault(); + // if submenu is open, activate the focused item + // if submenu is closed, open it and focus the first item + }, + ' ': (e) => { + // same as Enter + e.preventDefault(); + }, + Escape: (e) => { + // close all submenus + }, + Tab: (e) => { + // close + } + }; + + // support direct access keys + }, [isOpen, submenuItems.length, submenuActiveIndex]); + + useKeyDownHandlers(keyHandlers); + useEffect(() => { if (isActive && buttonRef.current) { buttonRef.current.focus(); @@ -118,6 +182,33 @@ function MenubarSubmenu({ return unregister; }, [id, registerItem]); + // reset submenu active index when submenu is closed + useEffect(() => { + if (!isOpen) { + setSubmenuActiveIndex(-1); + } + }, isOpen); + + const registerSubmenuItem = useCallback((submenuId) => { + setSubmenuItems((prev) => [...prev, submenuId]); + + return () => { + setSubmenuItems((prev) => + prev.filter((currentId) => currentId !== submenuId) + ); + }; + }, []); + + const subMenuContext = useMemo( + () => ({ + submenuItems, + submenuActiveIndex, + setSubmenuActiveIndex, + registerSubmenuItem + }), + [submenuItems, submenuActiveIndex, registerSubmenuItem] + ); + return (
  • - - {children} - + + + {children} + +
    • ); } diff --git a/client/components/Menubar/contexts.jsx b/client/components/Menubar/contexts.jsx index 733fed3ae..2ed8548a5 100644 --- a/client/components/Menubar/contexts.jsx +++ b/client/components/Menubar/contexts.jsx @@ -12,8 +12,10 @@ export const MenubarContext = createContext({ activeIndex: -1, setActiveIndex: () => {}, registerItem: () => {}, - menuItems: [], + menuItems: [] +}); +export const SubmenuContext = createContext({ // submenu state submenuActiveIndex: -1, setSubmenuActiveIndex: () => {}, From ef6a71c9ad2364e5951e4f409fb5d82d5508eda7 Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Fri, 17 Jan 2025 14:27:11 -0800 Subject: [PATCH 09/80] chore: code cleanup --- client/components/Menubar/Menubar.jsx | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/client/components/Menubar/Menubar.jsx b/client/components/Menubar/Menubar.jsx index 3ae2bc0b0..d3cfdd3c4 100644 --- a/client/components/Menubar/Menubar.jsx +++ b/client/components/Menubar/Menubar.jsx @@ -41,35 +41,25 @@ function Menubar({ children, className }) { // if submenu is closed, open it and focus the last item if (menuOpen === 'none') { toggleMenuOpen(menuItems[activeIndex]); - setSubmenuActiveIndex(submenuItems.length - 1); // focus last - } else { - setSubmenuActiveIndex( - (prev) => (prev - 1 + submenuItems.length) % submenuItems.length - ); } - // if submenu is open, focus the previous item }, ArrowDown: (e) => { e.preventDefault(); // if submenu is closed, open it and focus the first item if (menuOpen === 'none') { toggleMenuOpen(menuItems[activeIndex]); - setSubmenuActiveIndex(0); // focus first - } else { - setSubmenuActiveIndex((prev) => (prev + 1) % submenuItems.length); } - // if submenu is open, focus the next item }, ArrowLeft: (e) => { e.preventDefault(); + // focus the previous item, wrapping around if we reach the beginning const newIndex = (activeIndex - 1 + menuItems.length) % menuItems.length; setActiveIndex(newIndex); - // if submenu is open, close it, open the next one and focus the next top-level item + // if submenu is open, close it if (menuOpen !== 'none') { toggleMenuOpen(menuItems[activeIndex]); - setMenuOpen(menuItems[newIndex]); } }, ArrowRight: (e) => { @@ -77,10 +67,9 @@ function Menubar({ children, className }) { const newIndex = (activeIndex + 1) % menuItems.length; setActiveIndex(newIndex); - // if submenu is open, close it, open previous one and focus the previous top-level item + // close the current submenu if it's happen if (menuOpen !== 'none') { toggleMenuOpen(menuItems[activeIndex]); - setMenuOpen(menuItems[newIndex]); } }, Enter: (e) => { From c636ccf2eac1dd3e8a4fd1effa53daeff28961d1 Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Fri, 17 Jan 2025 14:28:52 -0800 Subject: [PATCH 10/80] refactor: moved logo outside menubar for proper keyboard navigation --- client/modules/IDE/components/Header/Nav.jsx | 58 +++++++++++--------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/client/modules/IDE/components/Header/Nav.jsx b/client/modules/IDE/components/Header/Nav.jsx index 36d151ec2..afc6f3e93 100644 --- a/client/modules/IDE/components/Header/Nav.jsx +++ b/client/modules/IDE/components/Header/Nav.jsx @@ -39,6 +39,10 @@ const Nav = ({ layout }) => { ) : ( <>
    +
    + +
    + @@ -87,19 +91,40 @@ const UserMenu = () => { return null; }; -const DashboardMenu = () => { +const Logo = () => { const { t } = useTranslation(); - const editorLink = useSelector(selectSketchPath); - return ( -
      -
    • + const user = useSelector((state) => state.user); + + if (user?.username) { + return ( + -
      • + + ); + } + + return ( +       + + + ); +}; + +const DashboardMenu = () => { + const { t } = useTranslation(); + const editorLink = useSelector(selectSketchPath); + return ( +
      • { return (
          -
        • - {user && user.username !== undefined ? ( - - - - ) : ( - - - - )} -
          • {t('Nav.File.New')} From 84202cea339d75b4cd4abec7c9c59400315668dd Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Fri, 17 Jan 2025 14:49:44 -0800 Subject: [PATCH 11/80] fix: fixed inconsistent hook rendering in MenubarSubmenu --- client/components/Menubar/MenubarSubmenu.jsx | 39 +++++++++----------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/client/components/Menubar/MenubarSubmenu.jsx b/client/components/Menubar/MenubarSubmenu.jsx index a12210c42..5ab200c87 100644 --- a/client/components/Menubar/MenubarSubmenu.jsx +++ b/client/components/Menubar/MenubarSubmenu.jsx @@ -114,47 +114,42 @@ function MenubarSubmenu({ const { activeIndex, menuItems, registerItem } = useContext(MenubarContext); const [submenuItems, setSubmenuItems] = useState([]); const [submenuActiveIndex, setSubmenuActiveIndex] = useState(-1); - const isActive = menuItems[activeIndex] === id; const buttonRef = useRef(null); + const isActive = menuItems[activeIndex] === id; const triggerRole = customTriggerRole || 'menuitem'; const listRole = customListRole || 'menu'; const hasPopup = listRole === 'listbox' ? 'listbox' : 'menu'; - const keyHandlers = useMemo(() => { - // we only want to create the handlers if the menu is open, - // otherwise return empty handlers - if (!isOpen) { - return {}; - } - - return { + const keyHandlers = useMemo( + () => ({ + // we only want to create the handlers if the menu is open, + // otherwise early return{ ArrowUp: (e) => { + if (!isOpen) return; e.preventDefault(); e.stopPropagation(); - setSubmenuActiveIndex((prev) => { - const newIndex = - (prev - 1 + submenuItems.length) % submenuItems.length; - return newIndex; - }); + setSubmenuActiveIndex( + (prev) => (prev - 1 + submenuItems.length) % submenuItems.length + ); }, ArrowDown: (e) => { + if (!isOpen) return; e.preventDefault(); e.stopPropagation(); - setSubmenuActiveIndex((prev) => { - const newIndex = (prev + 1) % submenuItems.length; - return newIndex; - }); + setSubmenuActiveIndex((prev) => (prev + 1) % submenuItems.length); }, Enter: (e) => { + if (!isOpen) return; e.preventDefault(); // if submenu is open, activate the focused item // if submenu is closed, open it and focus the first item }, ' ': (e) => { // same as Enter + if (!isOpen) return; e.preventDefault(); }, Escape: (e) => { @@ -163,10 +158,10 @@ function MenubarSubmenu({ Tab: (e) => { // close } - }; - - // support direct access keys - }, [isOpen, submenuItems.length, submenuActiveIndex]); + // support direct access keys + }), + [isOpen, submenuItems.length, submenuActiveIndex] + ); useKeyDownHandlers(keyHandlers); From 9fb7b9eabdf4a7e67e583e1b18b9f6dbe2620e49 Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Fri, 17 Jan 2025 15:07:21 -0800 Subject: [PATCH 12/80] fix: fixed useEffect getting a boolean instead of dependency array --- client/components/Menubar/MenubarSubmenu.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/components/Menubar/MenubarSubmenu.jsx b/client/components/Menubar/MenubarSubmenu.jsx index 5ab200c87..92c0402c3 100644 --- a/client/components/Menubar/MenubarSubmenu.jsx +++ b/client/components/Menubar/MenubarSubmenu.jsx @@ -182,7 +182,7 @@ function MenubarSubmenu({ if (!isOpen) { setSubmenuActiveIndex(-1); } - }, isOpen); + }, [isOpen]); const registerSubmenuItem = useCallback((submenuId) => { setSubmenuItems((prev) => [...prev, submenuId]); From ce789d71b0399898a509bdddb957fcfd1d91b1cf Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Sat, 18 Jan 2025 14:50:56 -0800 Subject: [PATCH 13/80] refactor: renamed variables in preparation of state management refactor --- client/components/Menubar/MenubarSubmenu.jsx | 44 ++++++++++---------- client/components/Menubar/contexts.jsx | 16 +++---- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/client/components/Menubar/MenubarSubmenu.jsx b/client/components/Menubar/MenubarSubmenu.jsx index 92c0402c3..3fbe40b31 100644 --- a/client/components/Menubar/MenubarSubmenu.jsx +++ b/client/components/Menubar/MenubarSubmenu.jsx @@ -111,12 +111,14 @@ function MenubarSubmenu({ ...props }) { const { isOpen, handlers } = useMenuProps(id); - const { activeIndex, menuItems, registerItem } = useContext(MenubarContext); - const [submenuItems, setSubmenuItems] = useState([]); - const [submenuActiveIndex, setSubmenuActiveIndex] = useState(-1); + const { oldActiveIndex, oldMenuItems, oldRegisterItem } = useContext( + MenubarContext + ); + const [oldSubmenuItems, setOldSubmenuItems] = useState([]); + const [oldSubmenuActiveIndex, setOldSubmenuActiveIndex] = useState(-1); const buttonRef = useRef(null); - const isActive = menuItems[activeIndex] === id; + const isActive = oldMenuItems[oldActiveIndex] === id; const triggerRole = customTriggerRole || 'menuitem'; const listRole = customListRole || 'menu'; const hasPopup = listRole === 'listbox' ? 'listbox' : 'menu'; @@ -130,8 +132,8 @@ function MenubarSubmenu({ e.preventDefault(); e.stopPropagation(); - setSubmenuActiveIndex( - (prev) => (prev - 1 + submenuItems.length) % submenuItems.length + setOldSubmenuActiveIndex( + (prev) => (prev - 1 + oldSubmenuItems.length) % oldSubmenuItems.length ); }, ArrowDown: (e) => { @@ -139,7 +141,7 @@ function MenubarSubmenu({ e.preventDefault(); e.stopPropagation(); - setSubmenuActiveIndex((prev) => (prev + 1) % submenuItems.length); + setOldSubmenuActiveIndex((prev) => (prev + 1) % oldSubmenuItems.length); }, Enter: (e) => { if (!isOpen) return; @@ -160,7 +162,7 @@ function MenubarSubmenu({ } // support direct access keys }), - [isOpen, submenuItems.length, submenuActiveIndex] + [isOpen, oldSubmenuItems.length, oldSubmenuActiveIndex] ); useKeyDownHandlers(keyHandlers); @@ -173,35 +175,35 @@ function MenubarSubmenu({ // register this menu item useEffect(() => { - const unregister = registerItem(id); + const unregister = oldRegisterItem(id); return unregister; - }, [id, registerItem]); + }, [id, oldRegisterItem]); // reset submenu active index when submenu is closed useEffect(() => { if (!isOpen) { - setSubmenuActiveIndex(-1); + setOldSubmenuActiveIndex(-1); } }, [isOpen]); - const registerSubmenuItem = useCallback((submenuId) => { - setSubmenuItems((prev) => [...prev, submenuId]); + const oldRegisterSubmenuItem = useCallback((submenuId) => { + setOldSubmenuItems((prev) => [...prev, submenuId]); return () => { - setSubmenuItems((prev) => + setOldSubmenuItems((prev) => prev.filter((currentId) => currentId !== submenuId) ); }; }, []); - const subMenuContext = useMemo( + const submenuContext = useMemo( () => ({ - submenuItems, - submenuActiveIndex, - setSubmenuActiveIndex, - registerSubmenuItem + oldSubmenuItems, + oldSubmenuActiveIndex, + setOldSubmenuActiveIndex, + oldRegisterSubmenuItem }), - [submenuItems, submenuActiveIndex, registerSubmenuItem] + [oldSubmenuItems, oldSubmenuActiveIndex, oldRegisterSubmenuItem] ); return ( @@ -216,7 +218,7 @@ function MenubarSubmenu({ {...handlers} {...props} /> - + {children} diff --git a/client/components/Menubar/contexts.jsx b/client/components/Menubar/contexts.jsx index 2ed8548a5..ae3930867 100644 --- a/client/components/Menubar/contexts.jsx +++ b/client/components/Menubar/contexts.jsx @@ -9,16 +9,16 @@ export const MenubarContext = createContext({ createMenuHandlers: () => ({}), createMenuItemHandlers: () => ({}), toggleMenuOpen: () => {}, - activeIndex: -1, - setActiveIndex: () => {}, - registerItem: () => {}, - menuItems: [] + oldActiveIndex: -1, + setOldActiveIndex: () => {}, + oldRegisterItem: () => {}, + oldMenuItems: [] }); export const SubmenuContext = createContext({ // submenu state - submenuActiveIndex: -1, - setSubmenuActiveIndex: () => {}, - registerSubmenuItem: () => {}, - submenuItems: [] + oldSubmenuActiveIndex: -1, + setOldSubmenuActiveIndex: () => {}, + oldRegisterSubmenuItem: () => {}, + oldSubmenuItems: [] }); From 63a6444f1cadb85c8cab2b413937d8187992ff1a Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Sat, 18 Jan 2025 14:51:42 -0800 Subject: [PATCH 14/80] feat: added usePrevious hook for obtaining prev state value --- client/common/usePrevious.js | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 client/common/usePrevious.js diff --git a/client/common/usePrevious.js b/client/common/usePrevious.js new file mode 100644 index 000000000..0f79ed91a --- /dev/null +++ b/client/common/usePrevious.js @@ -0,0 +1,11 @@ +import React from 'react'; + +export default function usePrevious(value) { + const ref = React.useRef(); + + React.useEffect(() => { + ref.current = value; + }, [value]); + + return ref.current; +} From dc62a3b81b329fe65e5e60151be1d29f8f21a182 Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Sat, 18 Jan 2025 14:52:10 -0800 Subject: [PATCH 15/80] refactor: migrated menu item collections from Array to Set --- client/components/Menubar/Menubar.jsx | 97 +++++++++++++++-------- client/components/Menubar/MenubarItem.jsx | 54 ++++++++----- 2 files changed, 98 insertions(+), 53 deletions(-) diff --git a/client/components/Menubar/Menubar.jsx b/client/components/Menubar/Menubar.jsx index d3cfdd3c4..d77a2788d 100644 --- a/client/components/Menubar/Menubar.jsx +++ b/client/components/Menubar/Menubar.jsx @@ -1,29 +1,42 @@ import PropTypes from 'prop-types'; -import React, { useCallback, useMemo, useRef, useState } from 'react'; +import React, { + useCallback, + useMemo, + useRef, + useState, + useEffect +} from 'react'; import useModalClose from '../../common/useModalClose'; import { MenuOpenContext, MenubarContext } from './contexts'; import useKeyDownHandlers from '../../common/useKeyDownHandlers'; +import usePrevious from '../../common/usePrevious'; function Menubar({ children, className }) { const [menuOpen, setMenuOpen] = useState('none'); - const [activeIndex, setActiveIndex] = useState(-1); - const [menuItems, setMenuItems] = useState([]); - const [submenuActiveIndex, setSubmenuActiveIndex] = useState(-1); - const [submenuItems, setSubmenuItems] = useState([]); + + const menuItems = useRef(new Set()).current; + const [activeIndex, setActiveIndex] = useState(0); + const prevIndex = usePrevious(activeIndex); + + // old state variables + const [oldActiveIndex, setOldActiveIndex] = useState(-1); + const [oldMenuItems, setOldMenuItems] = useState([]); + const [oldSubmenuActiveIndex, setOldSubmenuActiveIndex] = useState(-1); + const [oldSubmenuItems, setOldSubmenuItems] = useState([]); const timerRef = useRef(null); const nodeRef = useRef(null); - const registerItem = useCallback((id) => { - setMenuItems((prev) => [...prev, id]); + const oldRegisterItem = useCallback((id) => { + setOldMenuItems((prev) => [...prev, id]); return () => { - setMenuItems((prev) => prev.filter((item) => item !== id)); + setOldMenuItems((prev) => prev.filter((item) => item !== id)); }; }, []); - const registerSubmenuItem = useCallback((id) => { - setSubmenuItems((prev) => [...prev, id]); + const oldRegisterSubmenuItem = useCallback((id) => { + setOldSubmenuItems((prev) => [...prev, id]); return () => { - setSubmenuItems((prev) => prev.filter((item) => item !== id)); + setOldSubmenuItems((prev) => prev.filter((item) => item !== id)); }; }, []); @@ -40,50 +53,50 @@ function Menubar({ children, className }) { e.preventDefault(); // if submenu is closed, open it and focus the last item if (menuOpen === 'none') { - toggleMenuOpen(menuItems[activeIndex]); + toggleMenuOpen(oldMenuItems[oldActiveIndex]); } }, ArrowDown: (e) => { e.preventDefault(); // if submenu is closed, open it and focus the first item if (menuOpen === 'none') { - toggleMenuOpen(menuItems[activeIndex]); + toggleMenuOpen(oldMenuItems[oldActiveIndex]); } }, ArrowLeft: (e) => { e.preventDefault(); // focus the previous item, wrapping around if we reach the beginning const newIndex = - (activeIndex - 1 + menuItems.length) % menuItems.length; - setActiveIndex(newIndex); + (oldActiveIndex - 1 + oldMenuItems.length) % oldMenuItems.length; + setOldActiveIndex(newIndex); // if submenu is open, close it if (menuOpen !== 'none') { - toggleMenuOpen(menuItems[activeIndex]); + toggleMenuOpen(oldMenuItems[oldActiveIndex]); } }, ArrowRight: (e) => { e.preventDefault(); - const newIndex = (activeIndex + 1) % menuItems.length; - setActiveIndex(newIndex); + const newIndex = (oldActiveIndex + 1) % oldMenuItems.length; + setOldActiveIndex(newIndex); // close the current submenu if it's happen if (menuOpen !== 'none') { - toggleMenuOpen(menuItems[activeIndex]); + toggleMenuOpen(oldMenuItems[oldActiveIndex]); } }, Enter: (e) => { e.preventDefault(); // if submenu is open, activate the focused item // if submenu is closed, open it and focus the first item - toggleMenuOpen(menuItems[activeIndex]); + toggleMenuOpen(oldMenuItems[oldActiveIndex]); }, ' ': (e) => { // same as Enter e.preventDefault(); // if submenu is open, activate the focused item // if submenu is closed, open it and focus the first item - toggleMenuOpen(menuItems[activeIndex]); + toggleMenuOpen(oldMenuItems[oldActiveIndex]); }, Escape: (e) => { // close all submenus @@ -94,7 +107,7 @@ function Menubar({ children, className }) { } // support direct access keys }), - [menuItems, menuOpen, activeIndex, toggleMenuOpen] + [oldMenuItems, menuOpen, oldActiveIndex, toggleMenuOpen] ); useKeyDownHandlers(keyHandlers); @@ -113,10 +126,22 @@ function Menubar({ children, className }) { const handleBlur = useCallback(() => { timerRef.current = setTimeout(() => { setMenuOpen('none'); - setActiveIndex(-1); + setOldActiveIndex(-1); }, 10); }, [timerRef, setMenuOpen]); + useEffect(() => { + if (activeIndex !== prevIndex) { + const items = Array.from(menuItems); + const activeNode = items[activeIndex]?.firstChild; + const prevNode = items[prevIndex]?.firstChild; + + prevNode?.setAttribute('tabindex', '-1'); + activeNode?.setAttribute('tabindex', '0'); + activeNode.focus(); + } + }, [activeIndex, prevIndex, menuItems]); + const contextValue = useMemo( () => ({ createMenuHandlers: (menu) => ({ @@ -143,26 +168,28 @@ function Menubar({ children, className }) { } }), toggleMenuOpen, - activeIndex, - setActiveIndex, - registerItem, menuItems, - submenuActiveIndex, - setSubmenuActiveIndex, - registerSubmenuItem, - submenuItems + oldActiveIndex, + setOldActiveIndex, + oldRegisterItem, + oldMenuItems, + oldSubmenuActiveIndex, + setOldSubmenuActiveIndex, + oldRegisterSubmenuItem, + oldSubmenuItems }), [ menuOpen, toggleMenuOpen, clearHideTimeout, handleBlur, - activeIndex, - registerItem, menuItems, - submenuActiveIndex, - registerSubmenuItem, - submenuItems + oldActiveIndex, + oldRegisterItem, + oldMenuItems, + oldSubmenuActiveIndex, + oldRegisterSubmenuItem, + oldSubmenuItems ] ); diff --git a/client/components/Menubar/MenubarItem.jsx b/client/components/Menubar/MenubarItem.jsx index 6f0d85b75..59ad968f7 100644 --- a/client/components/Menubar/MenubarItem.jsx +++ b/client/components/Menubar/MenubarItem.jsx @@ -1,5 +1,5 @@ import PropTypes from 'prop-types'; -import React, { useEffect, useContext, useRef, useMemo } from 'react'; +import React, { useState, useEffect, useContext, useRef, useMemo } from 'react'; import ButtonOrLink from '../../common/ButtonOrLink'; import { MenubarContext, SubmenuContext, ParentMenuContext } from './contexts'; @@ -11,13 +11,17 @@ function MenubarItem({ selected, ...rest }) { - const submenuItemRef = useRef(null); + const [isFirstChild, setIsFirstChild] = useState(false); + const { createMenuItemHandlers, menuItems } = useContext(MenubarContext); + const oldSubmenuItemRef = useRef(null); + const menuItemRef = useRef(null); const parent = useContext(ParentMenuContext); - const { createMenuItemHandlers } = useContext(MenubarContext); - const { registerSubmenuItem, submenuActiveIndex, submenuItems } = useContext( - SubmenuContext - ); + const { + oldRegisterSubmenuItem, + oldSubmenuActiveIndex, + oldSubmenuItems + } = useContext(SubmenuContext); const handlers = useMemo(() => createMenuItemHandlers(parent), [ createMenuItemHandlers, @@ -30,18 +34,32 @@ function MenubarItem({ const role = customRole || 'menuitem'; const ariaSelected = role === 'option' ? { 'aria-selected': selected } : {}; - const isActive = submenuItems[submenuActiveIndex] === id; // is this item active in its own submenu? + const isActive = oldSubmenuItems[oldSubmenuActiveIndex] === id; // is this item active in its own submenu? useEffect(() => { - if (isActive && submenuItemRef.current) { - submenuItemRef.current.focus(); + if (isActive && oldSubmenuItemRef.current) { + oldSubmenuItemRef.current.focus(); } - }, [isActive, submenuItemRef]); + }, [isActive, oldSubmenuItemRef]); + + useEffect(() => { + const menuItemNode = menuItemRef.current; + if (menuItemNode) { + if (menuItems.size === 0) { + setIsFirstChild(true); + } + menuItems.add(menuItemNode); + } + + return () => { + menuItems.delete(menuItemNode); + }; + }, [menuItems]); useEffect(() => { - const unregister = registerSubmenuItem(id); + const unregister = oldRegisterSubmenuItem(id); return unregister; - }, [id, registerSubmenuItem]); + }, [id, oldRegisterSubmenuItem]); useEffect(() => { if (isActive) { @@ -49,21 +67,21 @@ function MenubarItem({ id, isActive, parent, - submenuActiveIndex, - element: submenuItemRef.current + oldSubmenuActiveIndex, + element: oldSubmenuItemRef.current }); } - }, [isActive, id, parent, submenuActiveIndex]); + }, [isActive, id, parent, oldSubmenuActiveIndex]); return ( -
        • +
          • From d3ccf166b3db0ef6476eb60e2eb632ab7716748c Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Tue, 21 Jan 2025 17:04:17 -0800 Subject: [PATCH 16/80] feat: added usePrevious hook for focus management --- client/modules/IDE/hooks/usePrevious.js | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 client/modules/IDE/hooks/usePrevious.js diff --git a/client/modules/IDE/hooks/usePrevious.js b/client/modules/IDE/hooks/usePrevious.js new file mode 100644 index 000000000..ae050ae1e --- /dev/null +++ b/client/modules/IDE/hooks/usePrevious.js @@ -0,0 +1,9 @@ +import React, { useEffect, useRef } from 'react'; + +export default function usePrevious(value) { + const ref = useRef(); + useEffect(() => { + ref.current = value; + }, [value]); + return ref.current; +} From f3555dc7f2eaa4badac67eac8ed63ac48e968d53 Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Tue, 21 Jan 2025 17:05:38 -0800 Subject: [PATCH 17/80] refactor: migrated collections tracking for top level components from array to set --- client/components/Menubar/Menubar.jsx | 59 +++++++++++++--- client/components/Menubar/MenubarSubmenu.jsx | 72 +++++++++++++++++++- 2 files changed, 122 insertions(+), 9 deletions(-) diff --git a/client/components/Menubar/Menubar.jsx b/client/components/Menubar/Menubar.jsx index d77a2788d..3135763fe 100644 --- a/client/components/Menubar/Menubar.jsx +++ b/client/components/Menubar/Menubar.jsx @@ -17,6 +17,7 @@ function Menubar({ children, className }) { const menuItems = useRef(new Set()).current; const [activeIndex, setActiveIndex] = useState(0); const prevIndex = usePrevious(activeIndex); + const [isFirstChild, setIsFirstChild] = useState(false); // old state variables const [oldActiveIndex, setOldActiveIndex] = useState(-1); @@ -40,6 +41,25 @@ function Menubar({ children, className }) { }; }, []); + const registerTopLevelItem = useCallback( + (ref) => { + const menuItemNode = ref.current; + + if (menuItemNode) { + if (menuItems.size === 0) { + setIsFirstChild(true); + } + menuItems.add(menuItemNode); + console.log('Menubar Register: ', menuItemNode.textContent); + } + + return () => { + menuItems.delete(menuItemNode); + }; + }, + [menuItems] + ); + const toggleMenuOpen = useCallback( (menu) => { setMenuOpen((prevState) => (prevState === menu ? 'none' : menu)); @@ -66,9 +86,12 @@ function Menubar({ children, className }) { ArrowLeft: (e) => { e.preventDefault(); // focus the previous item, wrapping around if we reach the beginning - const newIndex = - (oldActiveIndex - 1 + oldMenuItems.length) % oldMenuItems.length; - setOldActiveIndex(newIndex); + // const newIndex = + // (oldActiveIndex - 1 + oldMenuItems.length) % oldMenuItems.length; + // setOldActiveIndex(newIndex); + + const newIndex = (activeIndex - 1 + menuItems.size) % menuItems.size; + setActiveIndex(newIndex); // if submenu is open, close it if (menuOpen !== 'none') { @@ -77,8 +100,12 @@ function Menubar({ children, className }) { }, ArrowRight: (e) => { e.preventDefault(); - const newIndex = (oldActiveIndex + 1) % oldMenuItems.length; - setOldActiveIndex(newIndex); + // const newIndex = (oldActiveIndex + 1) % oldMenuItems.length; + // setOldActiveIndex(newIndex); + + const newIndex = (activeIndex + 1) % menuItems.size; + console.log(activeIndex, newIndex, menuItems.size); + setActiveIndex(newIndex); // close the current submenu if it's happen if (menuOpen !== 'none') { @@ -107,7 +134,14 @@ function Menubar({ children, className }) { } // support direct access keys }), - [oldMenuItems, menuOpen, oldActiveIndex, toggleMenuOpen] + [ + menuItems, + activeIndex, + oldMenuItems, + menuOpen, + oldActiveIndex, + toggleMenuOpen + ] ); useKeyDownHandlers(keyHandlers); @@ -133,8 +167,9 @@ function Menubar({ children, className }) { useEffect(() => { if (activeIndex !== prevIndex) { const items = Array.from(menuItems); - const activeNode = items[activeIndex]?.firstChild; - const prevNode = items[prevIndex]?.firstChild; + const activeNode = items[activeIndex]; + const prevNode = items[prevIndex]; + console.log(activeNode, prevNode); prevNode?.setAttribute('tabindex', '-1'); activeNode?.setAttribute('tabindex', '0'); @@ -169,6 +204,10 @@ function Menubar({ children, className }) { }), toggleMenuOpen, menuItems, + activeIndex, + setActiveIndex, + registerTopLevelItem, + isFirstChild, oldActiveIndex, setOldActiveIndex, oldRegisterItem, @@ -184,6 +223,10 @@ function Menubar({ children, className }) { clearHideTimeout, handleBlur, menuItems, + activeIndex, + setActiveIndex, + registerTopLevelItem, + isFirstChild, oldActiveIndex, oldRegisterItem, oldMenuItems, diff --git a/client/components/Menubar/MenubarSubmenu.jsx b/client/components/Menubar/MenubarSubmenu.jsx index 3fbe40b31..592ebc98a 100644 --- a/client/components/Menubar/MenubarSubmenu.jsx +++ b/client/components/Menubar/MenubarSubmenu.jsx @@ -40,6 +40,38 @@ export function useMenuProps(id) { const MenubarTrigger = React.forwardRef( ({ id, title, role, hasPopup, ...props }, ref) => { const { isOpen, handlers } = useMenuProps(id); + const { + menuItems, + activeIndex, + registerTopLevelItem, + isFirstChild + } = useContext(MenubarContext); + + // const isActive = menuItems[activeIndex] === id; // is this item active in its own submenu? + const isActive = useMemo(() => { + const items = Array.from(menuItems); + const activeNode = items[activeIndex]; + // console.log(`${items[activeIndex]?.id}, ${id}`); + console.log(`${activeNode}, ${ref.current}`); + return items[activeIndex]?.id === id; + }, [menuItems, activeIndex, id]); + + useEffect(() => { + const unregister = registerTopLevelItem(ref); + return unregister; + }, [menuItems, registerTopLevelItem]); + + useEffect(() => { + // oldSubmenuItemRef.current.focus(); + const items = Array.from(menuItems); + console.log( + `${items[activeIndex]}: ${isActive}, index: ${activeIndex}, ref: ${ref.current}, id: ${id}` + ); + + if (isActive && ref.current) { + ref.current.focus(); + } + }, [ref, isActive, activeIndex]); return (
        ); } @@ -227,7 +227,7 @@ Menubar.propTypes = { Menubar.defaultProps = { children: null, - className: 'nav' + className: 'nav__menubar' }; export default Menubar; diff --git a/client/modules/IDE/components/Header/Nav.jsx b/client/modules/IDE/components/Header/Nav.jsx index afc6f3e93..ebb7e6e9b 100644 --- a/client/modules/IDE/components/Header/Nav.jsx +++ b/client/modules/IDE/components/Header/Nav.jsx @@ -43,10 +43,12 @@ const Nav = ({ layout }) => { - - - - +
    ); @@ -124,7 +126,7 @@ const DashboardMenu = () => { const { t } = useTranslation(); const editorLink = useSelector(selectSketchPath); return ( - `; From cb8001f722d0f1da263d1d7ee8fb0f9ec12d8be4 Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Fri, 31 Jan 2025 22:36:49 -0800 Subject: [PATCH 78/80] chore: fixed lint warnings and small optimizations --- client/common/usePrevious.js | 6 +- client/components/Menubar/Menubar.jsx | 60 ++-- client/components/Menubar/Menubar.test.jsx | 1 - client/components/Menubar/MenubarItem.jsx | 10 +- client/components/Menubar/MenubarSubmenu.jsx | 276 +++++++++---------- client/components/Menubar/contexts.jsx | 1 - client/modules/IDE/components/Header/Nav.jsx | 2 +- client/modules/IDE/hooks/usePrevious.js | 9 - 8 files changed, 167 insertions(+), 198 deletions(-) delete mode 100644 client/modules/IDE/hooks/usePrevious.js diff --git a/client/common/usePrevious.js b/client/common/usePrevious.js index 0f79ed91a..ed46581cb 100644 --- a/client/common/usePrevious.js +++ b/client/common/usePrevious.js @@ -1,9 +1,9 @@ -import React from 'react'; +import { useEffect, useRef } from 'react'; export default function usePrevious(value) { - const ref = React.useRef(); + const ref = useRef(); - React.useEffect(() => { + useEffect(() => { ref.current = value; }, [value]); diff --git a/client/components/Menubar/Menubar.jsx b/client/components/Menubar/Menubar.jsx index b87b365e6..439aa0768 100644 --- a/client/components/Menubar/Menubar.jsx +++ b/client/components/Menubar/Menubar.jsx @@ -36,7 +36,6 @@ function Menubar({ children, className }) { const [activeIndex, setActiveIndex] = useState(0); const prevIndex = usePrevious(activeIndex); const [hasFocus, setHasFocus] = useState(false); - const [isFirstChild, setIsFirstChild] = useState(false); // refs for menu items and their ids const menuItems = useRef(new Set()).current; @@ -161,31 +160,38 @@ function Menubar({ children, className }) { }, [nodeRef]); // keyboard navigation - const keyHandlers = useMemo( - () => ({ - ArrowLeft: (e) => { - e.preventDefault(); - e.stopPropagation(); - prev(); - }, - ArrowRight: (e) => { - e.preventDefault(); - e.stopPropagation(); - next(); - }, - Escape: (e) => { - e.preventDefault(); - e.stopPropagation(); - close(); - }, - Tab: (e) => { - e.stopPropagation(); - // close - } - // to do: support direct access keys - }), - [menuItems, activeIndex, menuOpen] - ); + const keyHandlers = { + ArrowLeft: (e) => { + e.preventDefault(); + e.stopPropagation(); + prev(); + }, + ArrowRight: (e) => { + e.preventDefault(); + e.stopPropagation(); + next(); + }, + Escape: (e) => { + e.preventDefault(); + e.stopPropagation(); + close(); + }, + Tab: (e) => { + e.stopPropagation(); + // close + }, + Home: (e) => { + e.preventDefault(); + e.stopPropagation(); + first(); + }, + End: (e) => { + e.preventDefault(); + e.stopPropagation(); + last(); + } + // to do: support direct access keys + }; // focus the active menu item and set its tabindex useEffect(() => { @@ -242,7 +248,6 @@ function Menubar({ children, className }) { registerTopLevelItem, setMenuOpen, toggleMenuOpen, - isFirstChild, hasFocus, setHasFocus }), @@ -253,7 +258,6 @@ function Menubar({ children, className }) { registerTopLevelItem, menuOpen, toggleMenuOpen, - isFirstChild, hasFocus, setHasFocus, clearHideTimeout, diff --git a/client/components/Menubar/Menubar.test.jsx b/client/components/Menubar/Menubar.test.jsx index 294eaf712..0f78d2f54 100644 --- a/client/components/Menubar/Menubar.test.jsx +++ b/client/components/Menubar/Menubar.test.jsx @@ -89,7 +89,6 @@ describe('Menubar', () => { const openMenuItem = screen.getByRole('menuitem', { name: 'Open' }); const editMenuTrigger = screen.getByRole('menuitem', { name: 'Edit' }); - const tidyMenuItem = screen.getByRole('menuitem', { name: 'Tidy' }); fireEvent.click(fileMenuTrigger); expect(fileMenuTrigger).toHaveAttribute('aria-expanded', 'true'); diff --git a/client/components/Menubar/MenubarItem.jsx b/client/components/Menubar/MenubarItem.jsx index 8b314025a..be0ccacdc 100644 --- a/client/components/Menubar/MenubarItem.jsx +++ b/client/components/Menubar/MenubarItem.jsx @@ -58,8 +58,7 @@ function MenubarItem({ const { setSubmenuActiveIndex, submenuItems, - registerSubmenuItem, - isFirstChild + registerSubmenuItem } = useContext(SubmenuContext); const parent = useContext(ParentMenuContext); @@ -67,10 +66,7 @@ function MenubarItem({ const menuItemRef = useRef(null); // handlers from parent menu - const handlers = useMemo(() => createMenuItemHandlers(parent), [ - createMenuItemHandlers, - parent - ]); + const handlers = createMenuItemHandlers(parent); // role and aria-selected const role = customRole || 'menuitem'; @@ -100,7 +96,7 @@ function MenubarItem({ {...handlers} {...ariaSelected} role={role} - tabIndex={isFirstChild ? 0 : -1} + tabIndex={-1} id={id} /> diff --git a/client/components/Menubar/MenubarSubmenu.jsx b/client/components/Menubar/MenubarSubmenu.jsx index e91d38b23..37f9fbc57 100644 --- a/client/components/Menubar/MenubarSubmenu.jsx +++ b/client/components/Menubar/MenubarSubmenu.jsx @@ -40,8 +40,6 @@ export function useMenuProps(id) { /** * @component * @param {Object} props - * @param {string} props.id - The id of submenu the trigger button controls - * @param {string} props.title - The title of the trigger button * @param {string} [props.role='menuitem'] - The ARIA role of the trigger button * @param {string} [props.hasPopup='menu'] - The ARIA property that indicates the presence of a popup * @returns {JSX.Element} @@ -58,8 +56,6 @@ export function useMenuProps(id) { * > * */ -const MenubarTrigger = React.forwardRef( - ({ id, role, title, hasPopup, ...props }, ref) => { - const { isOpen, handlers } = useMenuProps(id); - const { - setActiveIndex, - menuItems, - registerTopLevelItem, - hasFocus, - toggleMenuOpen - } = useContext(MenubarContext); - const { first, last } = useContext(SubmenuContext); - - // update active index when mouse enters the trigger and the menu has focus - const handleMouseEnter = () => { - if (hasFocus) { - const items = Array.from(menuItems); - const index = items.findIndex((item) => item === ref.current); - - if (index !== -1) { - setActiveIndex(index); - } - } - }; +const MenubarTrigger = React.forwardRef(({ role, hasPopup, ...props }, ref) => { + const { + setActiveIndex, + menuItems, + registerTopLevelItem, + hasFocus + } = useContext(MenubarContext); + const { id, title, first, last } = useContext(SubmenuContext); + const { isOpen, handlers } = useMenuProps(id); + + // update active index when mouse enters the trigger and the menu has focus + const handleMouseEnter = () => { + if (hasFocus) { + const items = Array.from(menuItems); + const index = items.findIndex((item) => item === ref.current); - // keyboard handlers - const handleKeyDown = (e) => { - switch (e.key) { - case 'ArrowDown': - if (!isOpen) { - e.preventDefault(); - e.stopPropagation(); - first(); - } - break; - case 'ArrowUp': - if (!isOpen) { - e.preventDefault(); - e.stopPropagation(); - last(); - } - break; - case 'Enter': - case ' ': - if (!isOpen) { - e.preventDefault(); - e.stopPropagation(); - first(); - } - break; - default: - break; + if (index !== -1) { + setActiveIndex(index); } - }; + } + }; + + // keyboard handlers + const handleKeyDown = (e) => { + switch (e.key) { + case 'ArrowDown': + if (!isOpen) { + e.preventDefault(); + e.stopPropagation(); + first(); + } + break; + case 'ArrowUp': + if (!isOpen) { + e.preventDefault(); + e.stopPropagation(); + last(); + } + break; + case 'Enter': + case ' ': + if (!isOpen) { + e.preventDefault(); + e.stopPropagation(); + first(); + } + break; + default: + break; + } + }; - // register trigger with parent menubar - useEffect(() => { - const unregister = registerTopLevelItem(ref, id); - return unregister; - }, [menuItems, registerTopLevelItem]); - - return ( - - ); - } -); + // register trigger with parent menubar + useEffect(() => { + const unregister = registerTopLevelItem(ref, id); + return unregister; + }, [menuItems, registerTopLevelItem]); + + return ( + + ); +}); MenubarTrigger.propTypes = { - id: PropTypes.string.isRequired, role: PropTypes.string, - title: PropTypes.node.isRequired, hasPopup: PropTypes.oneOf(['menu', 'listbox', 'true']) }; @@ -172,9 +163,7 @@ MenubarTrigger.defaultProps = { * @component * @param {Object} props * @param {React.ReactNode} props.children - MenubarItems that should be rendered in the list - * @param {string} props.id - The unique id of the submenu * @param {string} [props.role='menu'] - The ARIA role of the list element - * @param {string} props.title - The title of the list element * @returns {JSX.Element} */ @@ -182,12 +171,14 @@ MenubarTrigger.defaultProps = { * MenubarList renders the container for menu items in a submenu. It provides context and handles ARIA roles. * * @example - * + * * ... elements * */ -function MenubarList({ children, id, role, title, ...props }) { +function MenubarList({ children, role, ...props }) { + const { id, title } = useContext(SubmenuContext); + return (
      * - * New + * New * Save * * @@ -254,8 +244,7 @@ function MenubarSubmenu({ // core state for submenu management const { isOpen, handlers } = useMenuProps(id); const [submenuActiveIndex, setSubmenuActiveIndex] = useState(0); - const [isFirstChild, setIsFirstChild] = useState(false); - const { menuOpen, setMenuOpen, toggleMenuOpen } = useContext(MenubarContext); + const { setMenuOpen, toggleMenuOpen } = useContext(MenubarContext); const submenuItems = useRef(new Set()).current; // refs for the button and list elements @@ -337,10 +326,6 @@ function MenubarSubmenu({ const submenuItemNode = ref.current; if (submenuItemNode) { - if (submenuItems.size === 0) { - setIsFirstChild(true); - } - submenuItems.add(submenuItemNode); } @@ -351,51 +336,48 @@ function MenubarSubmenu({ [submenuItems] ); - // memoized key handlers for submenu navigation - const keyHandlers = useMemo( - () => ({ - ArrowUp: (e) => { - if (!isOpen) return; - e.preventDefault(); - e.stopPropagation(); - prev(); - }, - ArrowDown: (e) => { - if (!isOpen) return; - e.preventDefault(); - e.stopPropagation(); - next(); - }, - Enter: (e) => { - if (!isOpen) return; - e.preventDefault(); - e.stopPropagation(); - activate(); - }, - ' ': (e) => { - // same as Enter - if (!isOpen) return; - e.preventDefault(); - e.stopPropagation(); - activate(); - }, - Escape: (e) => { - if (!isOpen) return; - e.preventDefault(); - e.stopPropagation(); - close(); - }, - Tab: (e) => { - // close - if (!isOpen) return; - // e.preventDefault(); - e.stopPropagation(); - setMenuOpen('none'); - } - // support direct access keys - }), - [isOpen, submenuItems, submenuActiveIndex] - ); + // key handlers for submenu navigation + const keyHandlers = { + ArrowUp: (e) => { + if (!isOpen) return; + e.preventDefault(); + e.stopPropagation(); + prev(); + }, + ArrowDown: (e) => { + if (!isOpen) return; + e.preventDefault(); + e.stopPropagation(); + next(); + }, + Enter: (e) => { + if (!isOpen) return; + e.preventDefault(); + e.stopPropagation(); + activate(); + }, + ' ': (e) => { + // same as Enter + if (!isOpen) return; + e.preventDefault(); + e.stopPropagation(); + activate(); + }, + Escape: (e) => { + if (!isOpen) return; + e.preventDefault(); + e.stopPropagation(); + close(); + }, + Tab: (e) => { + // close + if (!isOpen) return; + // e.preventDefault(); + e.stopPropagation(); + setMenuOpen('none'); + } + // support direct access keys + }; // our custom keydown handler const handleKeyDown = useCallback( @@ -448,20 +430,22 @@ function MenubarSubmenu({ const submenuContext = useMemo( () => ({ + id, + title, submenuItems, submenuActiveIndex, setSubmenuActiveIndex, registerSubmenuItem, - isFirstChild, first, last }), [ + id, + title, submenuItems, submenuActiveIndex, setSubmenuActiveIndex, registerSubmenuItem, - isFirstChild, first, last ] @@ -475,17 +459,13 @@ function MenubarSubmenu({ > - - {children} - + {children} ); diff --git a/client/components/Menubar/contexts.jsx b/client/components/Menubar/contexts.jsx index 7fb164a53..18cad6c36 100644 --- a/client/components/Menubar/contexts.jsx +++ b/client/components/Menubar/contexts.jsx @@ -1,4 +1,3 @@ -import { set } from 'lodash'; import { createContext } from 'react'; export const ParentMenuContext = createContext('none'); diff --git a/client/modules/IDE/components/Header/Nav.jsx b/client/modules/IDE/components/Header/Nav.jsx index ebb7e6e9b..4da1e13ff 100644 --- a/client/modules/IDE/components/Header/Nav.jsx +++ b/client/modules/IDE/components/Header/Nav.jsx @@ -145,7 +145,7 @@ const ProjectMenu = () => { const isUserOwner = useSelector(getIsUserOwner); const project = useSelector((state) => state.project); const user = useSelector((state) => state.user); - const userSketches = `/${user.username}/sketches`; + const isUnsaved = !project?.id; const rootFile = useSelector(selectRootFile); diff --git a/client/modules/IDE/hooks/usePrevious.js b/client/modules/IDE/hooks/usePrevious.js deleted file mode 100644 index ae050ae1e..000000000 --- a/client/modules/IDE/hooks/usePrevious.js +++ /dev/null @@ -1,9 +0,0 @@ -import React, { useEffect, useRef } from 'react'; - -export default function usePrevious(value) { - const ref = useRef(); - useEffect(() => { - ref.current = value; - }, [value]); - return ref.current; -} From fd425ae89d593c8a3525d6bc8aea1d9eb519557b Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Sat, 1 Feb 2025 01:30:08 -0800 Subject: [PATCH 79/80] chore: fixed lint warning --- client/components/Menubar/MenubarItem.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/components/Menubar/MenubarItem.jsx b/client/components/Menubar/MenubarItem.jsx index be0ccacdc..687541b74 100644 --- a/client/components/Menubar/MenubarItem.jsx +++ b/client/components/Menubar/MenubarItem.jsx @@ -1,5 +1,5 @@ import PropTypes from 'prop-types'; -import React, { useEffect, useContext, useRef, useMemo } from 'react'; +import React, { useEffect, useContext, useRef } from 'react'; import { MenubarContext, SubmenuContext, ParentMenuContext } from './contexts'; import ButtonOrLink from '../../common/ButtonOrLink'; From 866f6fa60f031d2661da180d028ccbe9a45d02f0 Mon Sep 17 00:00:00 2001 From: Tristan Espinoza Date: Wed, 5 Feb 2025 20:03:25 -0800 Subject: [PATCH 80/80] fix: removed 'title' from MenubarList proptypes --- client/components/Menubar/MenubarSubmenu.jsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/components/Menubar/MenubarSubmenu.jsx b/client/components/Menubar/MenubarSubmenu.jsx index 37f9fbc57..c62dbd8e5 100644 --- a/client/components/Menubar/MenubarSubmenu.jsx +++ b/client/components/Menubar/MenubarSubmenu.jsx @@ -195,8 +195,7 @@ function MenubarList({ children, role, ...props }) { MenubarList.propTypes = { children: PropTypes.node, - role: PropTypes.oneOf(['menu', 'listbox']), - title: PropTypes.string.isRequired + role: PropTypes.oneOf(['menu', 'listbox']) }; MenubarList.defaultProps = {