From a49e67353e3e69b81c3e0cf7cfdf4b6f0101f420 Mon Sep 17 00:00:00 2001 From: Diana Nanyanzi Date: Sun, 26 Jan 2025 02:58:40 +0300 Subject: [PATCH 01/22] fix: separate the dom and modal container keydown logic --- .../src/command-palette/command-palette.js | 23 +++++++++++++------ .../command-palette/hooks/use-navigation.js | 15 ++++-------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/components/header-bar/src/command-palette/command-palette.js b/components/header-bar/src/command-palette/command-palette.js index d1da6c279..ae1066ddc 100755 --- a/components/header-bar/src/command-palette/command-palette.js +++ b/components/header-bar/src/command-palette/command-palette.js @@ -19,10 +19,13 @@ import { const CommandPalette = ({ apps, commands, shortcuts }) => { const containerEl = useRef(null) - const [show, setShow] = useState(false) + const [openModal, setOpenModal] = useState(false) const { currentView, filter, setFilter } = useCommandPaletteContext() - const handleVisibilityToggle = useCallback(() => setShow(!show), [show]) + const handleVisibilityToggle = useCallback( + () => setOpenModal(!openModal), + [openModal] + ) const handleFilterChange = useCallback( ({ value }) => setFilter(value), [setFilter] @@ -38,9 +41,8 @@ const CommandPalette = ({ apps, commands, shortcuts }) => { } = useFilter({ apps, commands, shortcuts }) const { handleKeyDown, goToDefaultView, modalRef } = useNavigation({ - setShow, + setOpenModal, itemsArray: currentViewItemsArray, - show, showGrid: apps?.length > 0, actionsLength: actionsArray?.length, }) @@ -65,11 +67,17 @@ const CommandPalette = ({ apps, commands, shortcuts }) => { } useEffect(() => { + const handleKeyDown = (event) => { + if ((event.metaKey || event.ctrlKey) && event.key === '/') { + setOpenModal((open) => !open) + goToDefaultView() + } + } document.addEventListener('keydown', handleKeyDown) return () => { document.removeEventListener('keydown', handleKeyDown) } - }, [handleKeyDown]) + }, [goToDefaultView]) return (
@@ -79,14 +87,15 @@ const CommandPalette = ({ apps, commands, shortcuts }) => { > - {show ? ( - + {openModal ? ( +
{ @@ -180,7 +179,7 @@ export const useNavigation = ({ if (event.key === 'Escape') { event.preventDefault() - setShow(false) + setOpenModal(false) setActiveSection(defaultSection) setHighlightedIndex(0) } @@ -193,7 +192,7 @@ export const useNavigation = ({ highlightedIndex, setActiveSection, setHighlightedIndex, - setShow, + setOpenModal, ] ) @@ -219,11 +218,6 @@ export const useNavigation = ({ }) } - if ((event.metaKey || event.ctrlKey) && event.key === '/') { - setShow((show) => !show) - goToDefaultView() - } - if (event.key === 'Enter') { if (activeSection === 'actions') { modal @@ -246,8 +240,7 @@ export const useNavigation = ({ highlightedIndex, itemsArray, setActiveSection, - setShow, - show, + setOpenModal, showGrid, ] ) From 1ec35f996ca66e9e6ae52196350e9c980262f18a Mon Sep 17 00:00:00 2001 From: Diana Nanyanzi Date: Mon, 3 Feb 2025 11:35:37 +0300 Subject: [PATCH 02/22] fix: pass interactive handlers to modal container --- .../src/command-palette/command-palette.js | 17 +++++++-------- .../src/command-palette/sections/container.js | 21 +++++++++++++++++-- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/components/header-bar/src/command-palette/command-palette.js b/components/header-bar/src/command-palette/command-palette.js index ae1066ddc..dfb68d608 100755 --- a/components/header-bar/src/command-palette/command-palette.js +++ b/components/header-bar/src/command-palette/command-palette.js @@ -88,15 +88,14 @@ const CommandPalette = ({ apps, commands, shortcuts }) => { {openModal ? ( - -
+ +
{ +const ModalContainer = ({ + children, + setShow, + show, + modalRef, + onFocus, + onKeyDown, +}) => { return ( setShow(false)} translucent={show}> -
+
{children}
- + ) -} +}) ModalContainer.propTypes = { children: PropTypes.node, - modalRef: PropTypes.object, - setShow: PropTypes.func, - show: PropTypes.bool, onFocus: PropTypes.func, onKeyDown: PropTypes.func, } diff --git a/components/header-bar/src/command-palette/utils/constants.js b/components/header-bar/src/command-palette/utils/constants.js new file mode 100644 index 000000000..b346d64c3 --- /dev/null +++ b/components/header-bar/src/command-palette/utils/constants.js @@ -0,0 +1,15 @@ +// views +export const HOME_VIEW = 'HOME' +export const ALL_APPS_VIEW = 'APPS' +export const ALL_COMMANDS_VIEW = 'COMMANDS' +export const ALL_SHORTCUTS_VIEW = 'SHORTCUTS' + +// home section +export const GRID_SECTION = 'GRID' +export const ACTIONS_SECTION = 'ACTIONS' + +// action types +export const APPS = 'APPS' +export const COMMANDS = 'COMMANDS' +export const SHORTCUTS = 'SHORTCUTS' +export const LOGOUT = 'LOGOUT' diff --git a/components/header-bar/src/command-palette/views/home-view.js b/components/header-bar/src/command-palette/views/home-view.js index 89ef449d6..28c009593 100644 --- a/components/header-bar/src/command-palette/views/home-view.js +++ b/components/header-bar/src/command-palette/views/home-view.js @@ -8,6 +8,7 @@ import { useCommandPaletteContext } from '../context/command-palette-context.js' import AppItem from '../sections/app-item.js' import Heading from '../sections/heading.js' import ListItem from '../sections/list-item.js' +import { ACTIONS_SECTION, GRID_SECTION, LOGOUT } from '../utils/constants.js' import ListView from './list-view.js' function HomeView({ apps, commands, shortcuts, actions }) { @@ -51,11 +52,12 @@ function HomeView({ apps, commands, shortcuts, actions }) { path={defaultAction} img={icon} highlighted={ - activeSection === 'grid' && + activeSection === + GRID_SECTION && highlightedIndex === idx } handleMouseEnter={() => { - setActiveSection('grid') + setActiveSection(GRID_SECTION) setHighlightedIndex(idx) }} /> @@ -72,7 +74,7 @@ function HomeView({ apps, commands, shortcuts, actions }) { )} {/* actions menu */} - +
{ - setActiveSection('actions') + setActiveSection(ACTIONS_SECTION) setHighlightedIndex(index) }} /> From eb926639e9b87fb7e232d9abc3402312f0f2f145 Mon Sep 17 00:00:00 2001 From: Diana Nanyanzi Date: Tue, 4 Feb 2025 03:50:46 +0300 Subject: [PATCH 04/22] refactor: navigation hook logic --- .../__tests__/command-palette.test.js | 2 +- .../src/command-palette/command-palette.js | 4 +- .../src/command-palette/hooks/use-actions.js | 9 +- .../src/command-palette/hooks/use-grid.js | 62 ++++++++ .../hooks/use-list-navigation.js | 33 ++++ .../src/command-palette/hooks/use-modal.js | 6 +- .../command-palette/hooks/use-navigation.js | 148 +++++++----------- .../command-palette/hooks/use-view-handler.js | 25 +++ .../src/command-palette/utils/constants.js | 5 + 9 files changed, 198 insertions(+), 96 deletions(-) create mode 100644 components/header-bar/src/command-palette/hooks/use-grid.js create mode 100644 components/header-bar/src/command-palette/hooks/use-list-navigation.js create mode 100644 components/header-bar/src/command-palette/hooks/use-view-handler.js diff --git a/components/header-bar/src/command-palette/__tests__/command-palette.test.js b/components/header-bar/src/command-palette/__tests__/command-palette.test.js index d6ac84e42..94ba48203 100644 --- a/components/header-bar/src/command-palette/__tests__/command-palette.test.js +++ b/components/header-bar/src/command-palette/__tests__/command-palette.test.js @@ -4,7 +4,7 @@ import PropTypes from 'prop-types' import React from 'react' import CommandPalette from '../command-palette.js' import { CommandPaletteContextProvider } from '../context/command-palette-context.js' -import { MIN_APPS_NUM } from '../hooks/use-navigation.js' +import { MIN_APPS_NUM } from '../utils/constants.js' const CommandPaletteProviderWrapper = ({ children }) => { return ( diff --git a/components/header-bar/src/command-palette/command-palette.js b/components/header-bar/src/command-palette/command-palette.js index 7757ad895..15b5c0733 100755 --- a/components/header-bar/src/command-palette/command-palette.js +++ b/components/header-bar/src/command-palette/command-palette.js @@ -55,7 +55,7 @@ const CommandPalette = ({ apps, commands, shortcuts }) => { const handleVisibilityToggle = useCallback( () => setModalOpen((open) => !open), - [] + [setModalOpen] ) useEffect(() => { @@ -97,7 +97,7 @@ const CommandPalette = ({ apps, commands, shortcuts }) => { document.removeEventListener('keydown', handleKeyDown) document.removeEventListener('click', onDocClick) } - }, [goToDefaultView]) + }, [goToDefaultView, setModalOpen, modalRef]) return (
diff --git a/components/header-bar/src/command-palette/hooks/use-actions.js b/components/header-bar/src/command-palette/hooks/use-actions.js index 7a9a76e6f..3f1d099c1 100644 --- a/components/header-bar/src/command-palette/hooks/use-actions.js +++ b/components/header-bar/src/command-palette/hooks/use-actions.js @@ -7,8 +7,13 @@ import { } from '@dhis2/ui-icons' import React, { useMemo } from 'react' import i18n from '../../locales/index.js' -import { APPS, COMMANDS, LOGOUT, SHORTCUTS } from '../utils/constants.js' -import { MIN_APPS_NUM } from './use-navigation.js' +import { + APPS, + COMMANDS, + LOGOUT, + MIN_APPS_NUM, + SHORTCUTS, +} from '../utils/constants.js' export const useAvailableActions = ({ apps, shortcuts, commands }) => { const actions = useMemo(() => { diff --git a/components/header-bar/src/command-palette/hooks/use-grid.js b/components/header-bar/src/command-palette/hooks/use-grid.js new file mode 100644 index 000000000..56e9d097f --- /dev/null +++ b/components/header-bar/src/command-palette/hooks/use-grid.js @@ -0,0 +1,62 @@ +import { useMemo } from 'react' +import { useCommandPaletteContext } from '../context/command-palette-context.js' + +const useGrid = ({ rows, columns }) => { + const { highlightedIndex } = useCommandPaletteContext() + const rowIndexDifference = useMemo(() => columns - 1, [columns]) + + const firstIndexPerRowArray = useMemo(() => { + const arr = [] + for (let i = 0; i < rows; i++) { + arr.push(i * columns) + } + return arr + }, [rows, columns]) + + const lastIndexPerRowArray = useMemo(() => { + return firstIndexPerRowArray.map((index) => index + rowIndexDifference) + }, [firstIndexPerRowArray, rowIndexDifference]) + + const activeRow = useMemo(() => { + let row = 0 + for (let i = 0; i < rows; i++) { + if (highlightedIndex >= firstIndexPerRowArray[i]) { + row = i + } + } + return row + }, [rows, firstIndexPerRowArray, highlightedIndex]) + + const [rowFirstIndex, rowLastIndex] = useMemo(() => { + const row = activeRow + const rowFirstIndex = firstIndexPerRowArray[row] + const rowLastIndex = lastIndexPerRowArray[row] + + return [rowFirstIndex, rowLastIndex] + }, [activeRow, firstIndexPerRowArray, lastIndexPerRowArray]) + + const nextLeftIndex = useMemo(() => { + return highlightedIndex > rowFirstIndex + ? highlightedIndex - 1 + : rowLastIndex + }, [highlightedIndex, rowFirstIndex, rowLastIndex]) + + const nextRightIndex = useMemo(() => { + return highlightedIndex >= rowLastIndex + ? rowFirstIndex + : highlightedIndex + 1 + }, [highlightedIndex, rowFirstIndex, rowLastIndex]) + + const lastRowFirstIndex = useMemo(() => { + return firstIndexPerRowArray[rows - 1] + }, [firstIndexPerRowArray, rows]) + + return { + nextLeftIndex, + nextRightIndex, + lastRowFirstIndex, + rows, + } +} + +export default useGrid diff --git a/components/header-bar/src/command-palette/hooks/use-list-navigation.js b/components/header-bar/src/command-palette/hooks/use-list-navigation.js new file mode 100644 index 000000000..ee7e1e384 --- /dev/null +++ b/components/header-bar/src/command-palette/hooks/use-list-navigation.js @@ -0,0 +1,33 @@ +import { useCallback } from 'react' +import { useCommandPaletteContext } from '../context/command-palette-context.js' + +const useListNavigation = () => { + const { highlightedIndex, setHighlightedIndex } = useCommandPaletteContext() + + const handleListNavigation = useCallback( + ({ event, listLength }) => { + const lastIndex = listLength - 1 + switch (event.key) { + case 'ArrowDown': + event.preventDefault() + setHighlightedIndex( + highlightedIndex >= lastIndex ? 0 : highlightedIndex + 1 + ) + break + case 'ArrowUp': + event.preventDefault() + setHighlightedIndex( + highlightedIndex > 0 ? highlightedIndex - 1 : lastIndex + ) + break + default: + break + } + }, + [highlightedIndex, setHighlightedIndex] + ) + + return handleListNavigation +} + +export default useListNavigation diff --git a/components/header-bar/src/command-palette/hooks/use-modal.js b/components/header-bar/src/command-palette/hooks/use-modal.js index 0b6bf9ec5..d8cbabbbf 100644 --- a/components/header-bar/src/command-palette/hooks/use-modal.js +++ b/components/header-bar/src/command-palette/hooks/use-modal.js @@ -7,13 +7,13 @@ const useModal = (modalRef) => { if (modalRef.current) { modalRef.current.showModal() } - }, []) + }, [modalRef]) const handleCloseModal = useCallback(() => { if (modalRef.current) { modalRef.current.close() } - }, []) + }, [modalRef]) useEffect(() => { if (modalOpen) { @@ -21,7 +21,7 @@ const useModal = (modalRef) => { } else { handleCloseModal } - }, [modalOpen]) + }, [modalOpen, handleCloseModal, handleOpenModal]) return { modalOpen, diff --git a/components/header-bar/src/command-palette/hooks/use-navigation.js b/components/header-bar/src/command-palette/hooks/use-navigation.js index fa432e002..651d9939e 100644 --- a/components/header-bar/src/command-palette/hooks/use-navigation.js +++ b/components/header-bar/src/command-palette/hooks/use-navigation.js @@ -1,10 +1,16 @@ import { useCallback, useEffect, useRef } from 'react' import { useCommandPaletteContext } from '../context/command-palette-context.js' -import { ACTIONS_SECTION, GRID_SECTION, HOME_VIEW } from '../utils/constants.js' +import { + ACTIONS_SECTION, + GRID_COLUMNS, + GRID_ROWS, + GRID_SECTION, + HOME_VIEW, +} from '../utils/constants.js' +import useGrid from './use-grid.js' +import useListNavigation from './use-list-navigation.js' import useModal from './use-modal.js' - -export const GRID_ITEMS_LENGTH = 8 -export const MIN_APPS_NUM = GRID_ITEMS_LENGTH +import useViewHandler from './use-view-handler.js' export const useNavigation = ({ itemsArray, showGrid, actionsLength }) => { const modalRef = useRef(null) @@ -15,67 +21,51 @@ export const useNavigation = ({ itemsArray, showGrid, actionsLength }) => { filter, highlightedIndex, setHighlightedIndex, - setFilter, - setCurrentView, setActiveSection, } = useCommandPaletteContext() const { modalOpen, setModalOpen } = useModal(modalRef) + const defaultSection = showGrid ? GRID_SECTION : ACTIONS_SECTION + + const goToDefaultView = useViewHandler({ section: defaultSection }) + + const { nextLeftIndex, nextRightIndex, lastRowFirstIndex } = useGrid({ + columns: GRID_COLUMNS, + rows: GRID_ROWS, + }) + // highlight first item in filtered results useEffect(() => { setHighlightedIndex(0) }, [filter, setHighlightedIndex]) - const defaultSection = showGrid ? GRID_SECTION : ACTIONS_SECTION + const switchActiveSection = useCallback( + ({ section, index }) => { + setActiveSection(section) + setHighlightedIndex(index) + }, + [setActiveSection, setHighlightedIndex] + ) - const goToDefaultView = useCallback(() => { - setFilter('') - setCurrentView(HOME_VIEW) - setActiveSection(defaultSection) - setHighlightedIndex(0) - }, [ - setActiveSection, - setCurrentView, - setFilter, - setHighlightedIndex, - defaultSection, - ]) + const handleListNavigation = useListNavigation() const handleListViewNavigation = useCallback( ({ event, listLength }) => { - const lastIndex = listLength - 1 - switch (event.key) { - case 'ArrowDown': - event.preventDefault() - setHighlightedIndex( - highlightedIndex >= lastIndex ? 0 : highlightedIndex + 1 - ) - break - case 'ArrowUp': - event.preventDefault() - setHighlightedIndex( - highlightedIndex > 0 ? highlightedIndex - 1 : lastIndex - ) - break - case 'Escape': - event.preventDefault() - goToDefaultView() - break - default: - break + handleListNavigation({ event, listLength }) + + if (event.key === 'Escape') { + event.preventDefault() + goToDefaultView() } }, - [goToDefaultView, highlightedIndex, setHighlightedIndex] + [handleListNavigation, goToDefaultView] ) const handleHomeViewNavigation = useCallback( (event) => { // grid - const gridRowLength = GRID_ITEMS_LENGTH / 2 // 4 - const topRowLastIndex = gridRowLength - 1 // 3 - const lastRowFirstIndex = gridRowLength // 4 - const lastRowLastIndex = GRID_ITEMS_LENGTH - 1 // 7 + const verticalPositionGap = GRID_COLUMNS // actions const lastActionIndex = actionsLength - 1 @@ -85,60 +75,34 @@ export const useNavigation = ({ itemsArray, showGrid, actionsLength }) => { case 'ArrowLeft': event.preventDefault() if (activeSection === GRID_SECTION) { - // row 1 - if (highlightedIndex <= topRowLastIndex) { - setHighlightedIndex( - highlightedIndex > 0 - ? highlightedIndex - 1 - : topRowLastIndex - ) - } - // row 2 - if (highlightedIndex >= lastRowFirstIndex) { - setHighlightedIndex( - highlightedIndex > lastRowFirstIndex - ? highlightedIndex - 1 - : lastRowLastIndex - ) - } + setHighlightedIndex(nextLeftIndex) } break case 'ArrowRight': event.preventDefault() if (activeSection === GRID_SECTION) { - // row 1 - if (highlightedIndex <= topRowLastIndex) { - setHighlightedIndex( - highlightedIndex >= topRowLastIndex - ? 0 - : highlightedIndex + 1 - ) - } - // row 2 - if (highlightedIndex >= lastRowFirstIndex) { - setHighlightedIndex( - highlightedIndex >= lastRowLastIndex - ? lastRowFirstIndex - : highlightedIndex + 1 - ) - } + setHighlightedIndex(nextRightIndex) } break case 'ArrowDown': event.preventDefault() if (activeSection === GRID_SECTION) { if (highlightedIndex >= lastRowFirstIndex) { - setActiveSection(ACTIONS_SECTION) - setHighlightedIndex(0) + switchActiveSection({ + section: ACTIONS_SECTION, + index: 0, + }) } else { setHighlightedIndex( - highlightedIndex + gridRowLength + highlightedIndex + verticalPositionGap ) } } else if (activeSection === ACTIONS_SECTION) { if (highlightedIndex >= actionsLength - 1) { - setActiveSection(GRID_SECTION) - setHighlightedIndex(0) + switchActiveSection({ + section: GRID_SECTION, + index: 0, + }) } else { setHighlightedIndex(highlightedIndex + 1) } @@ -148,17 +112,21 @@ export const useNavigation = ({ itemsArray, showGrid, actionsLength }) => { event.preventDefault() if (activeSection === GRID_SECTION) { if (highlightedIndex < lastRowFirstIndex) { - setActiveSection(ACTIONS_SECTION) - setHighlightedIndex(lastActionIndex) + switchActiveSection({ + section: ACTIONS_SECTION, + index: lastActionIndex, + }) } else { setHighlightedIndex( - highlightedIndex - gridRowLength + highlightedIndex - verticalPositionGap ) } } else if (activeSection === ACTIONS_SECTION) { if (highlightedIndex <= 0) { - setActiveSection(GRID_SECTION) - setHighlightedIndex(lastRowFirstIndex) + switchActiveSection({ + section: GRID_SECTION, + index: lastRowFirstIndex, + }) } else { setHighlightedIndex(highlightedIndex - 1) } @@ -189,6 +157,12 @@ export const useNavigation = ({ itemsArray, showGrid, actionsLength }) => { defaultSection, handleListViewNavigation, highlightedIndex, + nextLeftIndex, + nextRightIndex, + lastRowFirstIndex, + setModalOpen, + showGrid, + switchActiveSection, setActiveSection, setHighlightedIndex, ] @@ -232,13 +206,11 @@ export const useNavigation = ({ itemsArray, showGrid, actionsLength }) => { activeSection, currentView, filter.length, - goToDefaultView, handleHomeViewNavigation, handleListViewNavigation, highlightedIndex, itemsArray, setActiveSection, - showGrid, ] ) diff --git a/components/header-bar/src/command-palette/hooks/use-view-handler.js b/components/header-bar/src/command-palette/hooks/use-view-handler.js new file mode 100644 index 000000000..cd1c36986 --- /dev/null +++ b/components/header-bar/src/command-palette/hooks/use-view-handler.js @@ -0,0 +1,25 @@ +import { useCallback } from 'react' +import { useCommandPaletteContext } from '../context/command-palette-context.js' +import { HOME_VIEW } from '../utils/constants.js' + +const useViewHandler = ({ section }) => { + const { setHighlightedIndex, setFilter, setCurrentView, setActiveSection } = + useCommandPaletteContext() + + const goToDefaultView = useCallback(() => { + setFilter('') + setCurrentView(HOME_VIEW) + setActiveSection(section) + setHighlightedIndex(0) + }, [ + setActiveSection, + setCurrentView, + setFilter, + setHighlightedIndex, + section, + ]) + + return goToDefaultView +} + +export default useViewHandler diff --git a/components/header-bar/src/command-palette/utils/constants.js b/components/header-bar/src/command-palette/utils/constants.js index b346d64c3..1004eb6b0 100644 --- a/components/header-bar/src/command-palette/utils/constants.js +++ b/components/header-bar/src/command-palette/utils/constants.js @@ -13,3 +13,8 @@ export const APPS = 'APPS' export const COMMANDS = 'COMMANDS' export const SHORTCUTS = 'SHORTCUTS' export const LOGOUT = 'LOGOUT' + +// grid +export const MIN_APPS_NUM = 8 +export const GRID_COLUMNS = 4 +export const GRID_ROWS = 2 From 801d3c7859bf845b70423640ae4dbed1e056544d Mon Sep 17 00:00:00 2001 From: Diana Nanyanzi Date: Tue, 4 Feb 2025 13:21:35 +0300 Subject: [PATCH 05/22] test: mock dialog component for tests --- .../__tests__/browse-apps-view.test.js | 14 ++++++++++++++ .../__tests__/browse-commands-view.test.js | 9 ++++++++- .../__tests__/browse-shortcuts-view.test.js | 6 ++++++ .../__tests__/command-palette.test.js | 6 ++++++ .../command-palette/__tests__/home-view.test.js | 6 ++++++ .../__tests__/search-results.test.js | 6 ++++++ 6 files changed, 46 insertions(+), 1 deletion(-) diff --git a/components/header-bar/src/command-palette/__tests__/browse-apps-view.test.js b/components/header-bar/src/command-palette/__tests__/browse-apps-view.test.js index aae336bd6..ced3f0dc8 100644 --- a/components/header-bar/src/command-palette/__tests__/browse-apps-view.test.js +++ b/components/header-bar/src/command-palette/__tests__/browse-apps-view.test.js @@ -11,6 +11,13 @@ import { } from './command-palette.test.js' describe('Command Palette - List View - Browse Apps View', () => { + beforeAll(() => { + // Testing environment does not support the component yet so it has to mocked + // linked issue: https://github.com/jsdom/jsdom/issues/3294 + HTMLDialogElement.prototype.showModal = jest.fn() + HTMLDialogElement.prototype.close = jest.fn() + }) + it('renders Browse Apps View', async () => { const user = userEvent.setup() const { @@ -60,6 +67,7 @@ describe('Command Palette - List View - Browse Apps View', () => { findByPlaceholderText, container, findByTestId, + // debug } = render( { expect(listItems[0].querySelector('span')).toHaveTextContent( 'Test App 1' ) + listItems[0].focus() + + // debug() await user.keyboard('{ArrowDown}') expect(listItems[0]).not.toHaveClass('highlighted') @@ -94,6 +105,7 @@ describe('Command Palette - List View - Browse Apps View', () => { expect(listItems[1].querySelector('span')).toHaveTextContent( 'Test App 2' ) + listItems[1].focus() await user.keyboard('{ArrowDown}') expect(listItems[1]).not.toHaveClass('highlighted') @@ -101,6 +113,7 @@ describe('Command Palette - List View - Browse Apps View', () => { expect(listItems[2].querySelector('span')).toHaveTextContent( 'Test App 3' ) + listItems[2].focus() await user.keyboard('{ArrowUp}') expect(listItems[2]).not.toHaveClass('highlighted') @@ -108,6 +121,7 @@ describe('Command Palette - List View - Browse Apps View', () => { expect(listItems[1].querySelector('span')).toHaveTextContent( 'Test App 2' ) + listItems[1].focus() // filter items view await user.type(searchField, 'Test App') diff --git a/components/header-bar/src/command-palette/__tests__/browse-commands-view.test.js b/components/header-bar/src/command-palette/__tests__/browse-commands-view.test.js index 918e0406b..16536202e 100644 --- a/components/header-bar/src/command-palette/__tests__/browse-commands-view.test.js +++ b/components/header-bar/src/command-palette/__tests__/browse-commands-view.test.js @@ -8,6 +8,12 @@ import { } from './command-palette.test.js' describe('Command Palette - List View - Browse Commands', () => { + beforeAll(() => { + // Testing environment does not support the component yet so it has to mocked + // linked issue: https://github.com/jsdom/jsdom/issues/3294 + HTMLDialogElement.prototype.showModal = jest.fn() + HTMLDialogElement.prototype.close = jest.fn() + }) it('renders Browse Commands View', async () => { const user = userEvent.setup() const { @@ -42,10 +48,11 @@ describe('Command Palette - List View - Browse Commands', () => { 'Test Command 1' ) expect(listItem).toHaveClass('highlighted') + listItem.focus() // Esc key goes back to default view await user.keyboard('{Escape}') - // expect(queryByText(/All Commands/i)).not.toBeInTheDocument() + expect(queryByText(/All Commands/i)).not.toBeInTheDocument() expect(queryByTestId('headerbar-actions-menu')).toBeInTheDocument() }) }) diff --git a/components/header-bar/src/command-palette/__tests__/browse-shortcuts-view.test.js b/components/header-bar/src/command-palette/__tests__/browse-shortcuts-view.test.js index 5b6c5c543..94b1e0f73 100644 --- a/components/header-bar/src/command-palette/__tests__/browse-shortcuts-view.test.js +++ b/components/header-bar/src/command-palette/__tests__/browse-shortcuts-view.test.js @@ -8,6 +8,12 @@ import { } from './command-palette.test.js' describe('Command Palette - List View - Browse Shortcuts', () => { + beforeAll(() => { + // Testing environment does not support the component yet so it has to mocked + // linked issue: https://github.com/jsdom/jsdom/issues/3294 + HTMLDialogElement.prototype.showModal = jest.fn() + HTMLDialogElement.prototype.close = jest.fn() + }) it('renders Browse Shortcuts View', async () => { const user = userEvent.setup() const { diff --git a/components/header-bar/src/command-palette/__tests__/command-palette.test.js b/components/header-bar/src/command-palette/__tests__/command-palette.test.js index 94ba48203..0cb4b3c81 100644 --- a/components/header-bar/src/command-palette/__tests__/command-palette.test.js +++ b/components/header-bar/src/command-palette/__tests__/command-palette.test.js @@ -53,6 +53,12 @@ export const testShortcuts = [ ] describe('Command Palette Component', () => { + beforeAll(() => { + // Testing environment does not support the component yet so it has to mocked + // linked issue: https://github.com/jsdom/jsdom/issues/3294 + HTMLDialogElement.prototype.showModal = jest.fn() + HTMLDialogElement.prototype.close = jest.fn() + }) it('renders bare default view when Command Palette is opened', async () => { const user = userEvent.setup() const { getByTestId, queryByTestId, getByPlaceholderText } = render( diff --git a/components/header-bar/src/command-palette/__tests__/home-view.test.js b/components/header-bar/src/command-palette/__tests__/home-view.test.js index a8854ffdd..931aaace1 100644 --- a/components/header-bar/src/command-palette/__tests__/home-view.test.js +++ b/components/header-bar/src/command-palette/__tests__/home-view.test.js @@ -12,6 +12,12 @@ import { } from './command-palette.test.js' describe('Command Palette - Home View', () => { + beforeAll(() => { + // Testing environment does not support the component yet so it has to mocked + // linked issue: https://github.com/jsdom/jsdom/issues/3294 + HTMLDialogElement.prototype.showModal = jest.fn() + HTMLDialogElement.prototype.close = jest.fn() + }) it('shows the full default view upon opening the Command Palette', async () => { const user = userEvent.setup() const { diff --git a/components/header-bar/src/command-palette/__tests__/search-results.test.js b/components/header-bar/src/command-palette/__tests__/search-results.test.js index 164dea918..11b89e406 100644 --- a/components/header-bar/src/command-palette/__tests__/search-results.test.js +++ b/components/header-bar/src/command-palette/__tests__/search-results.test.js @@ -11,6 +11,12 @@ import { } from './command-palette.test.js' describe('Command Palette - List View - Search Results', () => { + beforeAll(() => { + // Testing environment does not support the component yet so it has to mocked + // linked issue: https://github.com/jsdom/jsdom/issues/3294 + HTMLDialogElement.prototype.showModal = jest.fn() + HTMLDialogElement.prototype.close = jest.fn() + }) it('filters for one item and handles navigation of singular item list', async () => { const user = userEvent.setup() const { getByPlaceholderText, queryAllByTestId, container } = render( From d3114bbadf368541a6a3c956428adae6c9e86b30 Mon Sep 17 00:00:00 2001 From: Diana Nanyanzi Date: Tue, 4 Feb 2025 16:22:08 +0300 Subject: [PATCH 06/22] refactor: modal and search component fixes --- .../src/command-palette/command-palette.js | 67 ++++--------------- .../context/command-palette-context.js | 32 +++++---- .../src/command-palette/hooks/use-modal.js | 2 +- .../command-palette/hooks/use-navigation.js | 16 ++--- .../command-palette/hooks/use-view-handler.js | 37 +++++----- .../src/command-palette/sections/container.js | 40 ----------- .../sections/modal-container.js | 65 ++++++++++++++++++ .../command-palette/sections/search-field.js | 42 ++++++++---- 8 files changed, 156 insertions(+), 145 deletions(-) delete mode 100644 components/header-bar/src/command-palette/sections/container.js create mode 100644 components/header-bar/src/command-palette/sections/modal-container.js diff --git a/components/header-bar/src/command-palette/command-palette.js b/components/header-bar/src/command-palette/command-palette.js index 15b5c0733..092f23acf 100755 --- a/components/header-bar/src/command-palette/command-palette.js +++ b/components/header-bar/src/command-palette/command-palette.js @@ -2,14 +2,14 @@ import { colors, spacers } from '@dhis2/ui-constants' import { IconApps24 } from '@dhis2/ui-icons' import PropTypes from 'prop-types' import React, { useCallback, useRef, useEffect } from 'react' -import i18n from '../locales/index.js' import { useCommandPaletteContext } from './context/command-palette-context.js' import { useAvailableActions } from './hooks/use-actions.js' import { useFilter } from './hooks/use-filter.js' import { useNavigation } from './hooks/use-navigation.js' +import useViewAndSectionHandler from './hooks/use-view-handler.js' import BackButton from './sections/back-button.js' -import ModalContainer from './sections/container.js' -import Search from './sections/search-field.js' +import ModalContainer from './sections/modal-container.js' +import SearchFilter from './sections/search-field.js' import { ALL_APPS_VIEW, ALL_COMMANDS_VIEW, @@ -25,12 +25,7 @@ import { const CommandPalette = ({ apps, commands, shortcuts }) => { const containerEl = useRef(null) - const { currentView, filter, setFilter } = useCommandPaletteContext() - - const handleFilterChange = useCallback( - ({ value }) => setFilter(value), - [setFilter] - ) + const { currentView, filter, setShowGrid } = useCommandPaletteContext() const actionsArray = useAvailableActions({ apps, shortcuts, commands }) @@ -41,42 +36,22 @@ const CommandPalette = ({ apps, commands, shortcuts }) => { currentViewItemsArray, } = useFilter({ apps, commands, shortcuts }) - const { - handleKeyDown, - goToDefaultView, - modalRef, - setModalOpen, - showModal, - } = useNavigation({ + useEffect(() => { + setShowGrid(apps?.length > 0) + }, [apps, setShowGrid]) + + const { handleKeyDown, modalRef, setModalOpen, showModal } = useNavigation({ itemsArray: currentViewItemsArray, - showGrid: apps?.length > 0, actionsLength: actionsArray?.length, }) + const { goToDefaultView } = useViewAndSectionHandler() + const handleVisibilityToggle = useCallback( () => setModalOpen((open) => !open), [setModalOpen] ) - useEffect(() => { - const activeItem = document.querySelector('.highlighted') - if (activeItem && typeof activeItem.scrollIntoView === 'function') { - activeItem?.scrollIntoView({ behavior: 'smooth', block: 'nearest' }) - } - }) - - useEffect(() => { - if (modalRef.current) { - modalRef.current?.focus() - } - }) - - const handleFocus = (event) => { - if (event.target === modalRef?.current) { - modalRef.current?.querySelector('input').focus() - } - } - useEffect(() => { const handleKeyDown = (event) => { if ((event.metaKey || event.ctrlKey) && event.key === '/') { @@ -108,25 +83,9 @@ const CommandPalette = ({ apps, commands, shortcuts }) => { {showModal ? ( - +
- +
{currentView !== HOME_VIEW && !filter ? ( diff --git a/components/header-bar/src/command-palette/context/command-palette-context.js b/components/header-bar/src/command-palette/context/command-palette-context.js index aaf460052..858e78ad9 100644 --- a/components/header-bar/src/command-palette/context/command-palette-context.js +++ b/components/header-bar/src/command-palette/context/command-palette-context.js @@ -1,5 +1,5 @@ import PropTypes from 'prop-types' -import React, { createContext, useContext, useState } from 'react' +import React, { createContext, useContext, useMemo, useState } from 'react' import { HOME_VIEW } from '../utils/constants.js' const commandPaletteContext = createContext() @@ -10,20 +10,26 @@ export const CommandPaletteContextProvider = ({ children }) => { const [currentView, setCurrentView] = useState(HOME_VIEW) // home view sections const [activeSection, setActiveSection] = useState(null) + const [showGrid, setShowGrid] = useState(null) + + const contextValue = useMemo( + () => ({ + filter, + setFilter, + highlightedIndex, + setHighlightedIndex, + currentView, + setCurrentView, + activeSection, + setActiveSection, + showGrid, + setShowGrid, + }), + [filter, highlightedIndex, currentView, activeSection, showGrid] + ) return ( - + {children} ) diff --git a/components/header-bar/src/command-palette/hooks/use-modal.js b/components/header-bar/src/command-palette/hooks/use-modal.js index d8cbabbbf..cc80c718a 100644 --- a/components/header-bar/src/command-palette/hooks/use-modal.js +++ b/components/header-bar/src/command-palette/hooks/use-modal.js @@ -19,7 +19,7 @@ const useModal = (modalRef) => { if (modalOpen) { handleOpenModal() } else { - handleCloseModal + handleCloseModal() } }, [modalOpen, handleCloseModal, handleOpenModal]) diff --git a/components/header-bar/src/command-palette/hooks/use-navigation.js b/components/header-bar/src/command-palette/hooks/use-navigation.js index 651d9939e..0b82db90b 100644 --- a/components/header-bar/src/command-palette/hooks/use-navigation.js +++ b/components/header-bar/src/command-palette/hooks/use-navigation.js @@ -10,9 +10,9 @@ import { import useGrid from './use-grid.js' import useListNavigation from './use-list-navigation.js' import useModal from './use-modal.js' -import useViewHandler from './use-view-handler.js' +import useViewAndSectionHandler from './use-view-handler.js' -export const useNavigation = ({ itemsArray, showGrid, actionsLength }) => { +export const useNavigation = ({ itemsArray, actionsLength }) => { const modalRef = useRef(null) const { @@ -22,13 +22,12 @@ export const useNavigation = ({ itemsArray, showGrid, actionsLength }) => { highlightedIndex, setHighlightedIndex, setActiveSection, + showGrid, } = useCommandPaletteContext() const { modalOpen, setModalOpen } = useModal(modalRef) - const defaultSection = showGrid ? GRID_SECTION : ACTIONS_SECTION - - const goToDefaultView = useViewHandler({ section: defaultSection }) + const { goToDefaultSection, goToDefaultView } = useViewAndSectionHandler() const { nextLeftIndex, nextRightIndex, lastRowFirstIndex } = useGrid({ columns: GRID_COLUMNS, @@ -147,14 +146,12 @@ export const useNavigation = ({ itemsArray, showGrid, actionsLength }) => { if (event.key === 'Escape') { event.preventDefault() setModalOpen(false) - setActiveSection(defaultSection) - setHighlightedIndex(0) + goToDefaultSection() } }, [ activeSection, actionsLength, - defaultSection, handleListViewNavigation, highlightedIndex, nextLeftIndex, @@ -163,8 +160,8 @@ export const useNavigation = ({ itemsArray, showGrid, actionsLength }) => { setModalOpen, showGrid, switchActiveSection, - setActiveSection, setHighlightedIndex, + goToDefaultSection, ] ) @@ -216,7 +213,6 @@ export const useNavigation = ({ itemsArray, showGrid, actionsLength }) => { return { handleKeyDown, - goToDefaultView, modalRef, setModalOpen, showModal: modalOpen, diff --git a/components/header-bar/src/command-palette/hooks/use-view-handler.js b/components/header-bar/src/command-palette/hooks/use-view-handler.js index cd1c36986..6bdd6ba54 100644 --- a/components/header-bar/src/command-palette/hooks/use-view-handler.js +++ b/components/header-bar/src/command-palette/hooks/use-view-handler.js @@ -1,25 +1,32 @@ import { useCallback } from 'react' import { useCommandPaletteContext } from '../context/command-palette-context.js' -import { HOME_VIEW } from '../utils/constants.js' +import { ACTIONS_SECTION, GRID_SECTION, HOME_VIEW } from '../utils/constants.js' -const useViewHandler = ({ section }) => { - const { setHighlightedIndex, setFilter, setCurrentView, setActiveSection } = - useCommandPaletteContext() +const useViewAndSectionHandler = () => { + const { + setHighlightedIndex, + setFilter, + setCurrentView, + setActiveSection, + showGrid, + } = useCommandPaletteContext() + + const goToDefaultSection = useCallback(() => { + const defaultSection = showGrid ? GRID_SECTION : ACTIONS_SECTION + setActiveSection(defaultSection) + setHighlightedIndex(0) + }, [showGrid, setActiveSection, setHighlightedIndex]) const goToDefaultView = useCallback(() => { setFilter('') setCurrentView(HOME_VIEW) - setActiveSection(section) - setHighlightedIndex(0) - }, [ - setActiveSection, - setCurrentView, - setFilter, - setHighlightedIndex, - section, - ]) + goToDefaultSection() + }, [setCurrentView, setFilter, goToDefaultSection]) - return goToDefaultView + return { + goToDefaultView, + goToDefaultSection, + } } -export default useViewHandler +export default useViewAndSectionHandler diff --git a/components/header-bar/src/command-palette/sections/container.js b/components/header-bar/src/command-palette/sections/container.js deleted file mode 100644 index 36d4c3203..000000000 --- a/components/header-bar/src/command-palette/sections/container.js +++ /dev/null @@ -1,40 +0,0 @@ -import { colors, elevations } from '@dhis2/ui-constants' -import PropTypes from 'prop-types' -import React, { forwardRef } from 'react' - -const ModalContainer = forwardRef(function ModalContainer( - { children, onFocus, onKeyDown }, - ref -) { - return ( - <> - - {children} - - - - ) -}) - -ModalContainer.propTypes = { - children: PropTypes.node, - onFocus: PropTypes.func, - onKeyDown: PropTypes.func, -} - -export default ModalContainer diff --git a/components/header-bar/src/command-palette/sections/modal-container.js b/components/header-bar/src/command-palette/sections/modal-container.js new file mode 100644 index 000000000..1dc24a490 --- /dev/null +++ b/components/header-bar/src/command-palette/sections/modal-container.js @@ -0,0 +1,65 @@ +import { colors, elevations } from '@dhis2/ui-constants' +import PropTypes from 'prop-types' +import React, { forwardRef, useEffect } from 'react' + +const ModalContainer = forwardRef(function ModalContainer( + { children, onKeyDown }, + ref +) { + useEffect(() => { + const activeItem = ref?.current?.querySelector('.highlighted') + if (activeItem && typeof activeItem.scrollIntoView === 'function') { + activeItem?.scrollIntoView({ behavior: 'smooth', block: 'nearest' }) + } + }, [ref]) + + useEffect(() => { + if (!ref.current) { + return + } + const dialog = ref.current + + const handleFocus = (event) => { + if (event.target === ref?.current) { + ref.current?.querySelector('input').focus() + } + } + + dialog.addEventListener('focus', handleFocus) + dialog.addEventListener('keydown', onKeyDown) + + return () => { + dialog.removeEventListener('focus', handleFocus) + dialog.removeEventListener('keydown', onKeyDown) + } + }, [onKeyDown, ref]) + + return ( + <> + {children} + + + ) +}) + +ModalContainer.propTypes = { + children: PropTypes.node, + onKeyDown: PropTypes.func, +} + +export default ModalContainer diff --git a/components/header-bar/src/command-palette/sections/search-field.js b/components/header-bar/src/command-palette/sections/search-field.js index 6ea20c4cc..e85f8bd6e 100644 --- a/components/header-bar/src/command-palette/sections/search-field.js +++ b/components/header-bar/src/command-palette/sections/search-field.js @@ -1,17 +1,41 @@ import { colors, spacers, theme } from '@dhis2/ui-constants' import { IconSearch16 } from '@dhis2/ui-icons' -import PropTypes from 'prop-types' -import React from 'react' +import React, { useCallback, useMemo } from 'react' import { InputField } from '../../../../input/src/input-field/input-field.js' +import i18n from '../../locales/index.js' +import { useCommandPaletteContext } from '../context/command-palette-context.js' +import { + ALL_APPS_VIEW, + ALL_COMMANDS_VIEW, + ALL_SHORTCUTS_VIEW, +} from '../utils/constants.js' + +function SearchFilter() { + const { currentView, filter, setFilter } = useCommandPaletteContext() + + const handleFilterChange = useCallback( + ({ value }) => setFilter(value), + [setFilter] + ) + + const placeholder = useMemo(() => { + if (currentView === ALL_APPS_VIEW) { + return i18n.t('Search apps') + } else if (currentView === ALL_COMMANDS_VIEW) { + return i18n.t('Search commands') + } else if (currentView === ALL_SHORTCUTS_VIEW) { + return i18n.t('Search shortcuts') + } + return i18n.t('Search apps, shortcuts, commands') + }, [currentView]) -function Search({ value, onChange, placeholder }) { return ( <> Date: Tue, 4 Feb 2025 18:07:41 +0300 Subject: [PATCH 07/22] test: fix test when user clicks outside the modal --- .../the_app_menu_closes_when_the_user_clicks_outside.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/header-bar/src/features/the_headerbar_contains_a_menu_to_all_apps_shortcuts_and_commands/the_app_menu_closes_when_the_user_clicks_outside.js b/components/header-bar/src/features/the_headerbar_contains_a_menu_to_all_apps_shortcuts_and_commands/the_app_menu_closes_when_the_user_clicks_outside.js index ed0803bcc..5cbdd662b 100644 --- a/components/header-bar/src/features/the_headerbar_contains_a_menu_to_all_apps_shortcuts_and_commands/the_app_menu_closes_when_the_user_clicks_outside.js +++ b/components/header-bar/src/features/the_headerbar_contains_a_menu_to_all_apps_shortcuts_and_commands/the_app_menu_closes_when_the_user_clicks_outside.js @@ -5,5 +5,5 @@ When('the user opens the menu', () => { }) When('the user clicks outside of the menu', () => { - cy.get('.backdrop').click({ force: true }) + cy.get('.headerbar > dialog').click({ force: true }) }) From 478f7226c082fc461d440c14e74e69145ab6a836 Mon Sep 17 00:00:00 2001 From: Diana Nanyanzi Date: Tue, 4 Feb 2025 19:46:02 +0300 Subject: [PATCH 08/22] fix: remove focus by hovering - remove functionality creating focus on 2 items at once by keyboard and hovering - update test simulating focus by hovering over an item --- .../command-palette/__tests__/browse-apps-view.test.js | 8 ++++---- .../src/command-palette/sections/app-item.js | 10 ++-------- .../src/command-palette/sections/list-item.js | 6 ++---- .../header-bar/src/command-palette/sections/list.js | 3 +-- .../header-bar/src/command-palette/views/home-view.js | 9 --------- .../header-bar/src/command-palette/views/list-view.js | 3 ++- 6 files changed, 11 insertions(+), 28 deletions(-) diff --git a/components/header-bar/src/command-palette/__tests__/browse-apps-view.test.js b/components/header-bar/src/command-palette/__tests__/browse-apps-view.test.js index ced3f0dc8..6e2087da2 100644 --- a/components/header-bar/src/command-palette/__tests__/browse-apps-view.test.js +++ b/components/header-bar/src/command-palette/__tests__/browse-apps-view.test.js @@ -1,5 +1,5 @@ import { fireEvent } from '@testing-library/react' -import userEvent from '@testing-library/user-event' +import { userEvent } from '@testing-library/user-event' import React from 'react' import CommandPalette from '../command-palette.js' import { @@ -136,10 +136,10 @@ describe('Command Palette - List View - Browse Apps View', () => { 'Test App 1' ) - // simulate hover + // simulate hover - no highlight await user.hover(listItems[8]) - expect(listItems[1]).not.toHaveClass('highlighted') - expect(listItems[8]).toHaveClass('highlighted') + expect(listItems[0]).toHaveClass('highlighted') + expect(listItems[8]).not.toHaveClass('highlighted') expect(listItems[8].querySelector('span')).toHaveTextContent( 'Test App 9' ) diff --git a/components/header-bar/src/command-palette/sections/app-item.js b/components/header-bar/src/command-palette/sections/app-item.js index a258d346a..3fcf06faa 100644 --- a/components/header-bar/src/command-palette/sections/app-item.js +++ b/components/header-bar/src/command-palette/sections/app-item.js @@ -3,14 +3,9 @@ import cx from 'classnames' import PropTypes from 'prop-types' import React from 'react' -function AppItem({ name, path, img, highlighted, handleMouseEnter }) { +function AppItem({ name, path, img, highlighted }) { return ( - + app {name}