refactor: command palette UX and navigation (#1661)

- Use native dialog element for modal
- Refactor navigation logic
- Fix conflicts with focus and highlighting
- Add searchable logout action
- Change command palette shortcut trigger
- Update Esc, Tab and Backspace key handlers
- Open apps in the same window

---------

Co-authored-by: Joseph John Aas Cooper <33054985+cooper-joe@users.noreply.github.com>

collections/forms/i18n/en.pot

@@ -5,8 +5,8 @@ msgstr ""
 "Content-Type: text/plain; charset=utf-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=2; plural=(n != 1)\n"
-"POT-Creation-Date: 2024-11-11T07:36:10.587Z\n"
-"PO-Revision-Date: 2024-11-11T07:36:10.588Z\n"
+"POT-Creation-Date: 2025-02-13T16:05:53.251Z\n"
+"PO-Revision-Date: 2025-02-13T16:05:53.254Z\n"
 
 msgid "Upload file"
 msgstr "Upload file"

components/header-bar/i18n/en.pot

@@ -5,12 +5,48 @@ msgstr ""
 "Content-Type: text/plain; charset=utf-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=2; plural=(n != 1)\n"
-"POT-Creation-Date: 2024-06-12T03:40:49.012Z\n"
-"PO-Revision-Date: 2024-06-12T03:40:49.013Z\n"
+"POT-Creation-Date: 2025-02-13T15:42:36.303Z\n"
+"PO-Revision-Date: 2025-02-13T15:42:36.304Z\n"
+
+msgid "Browse apps"
+msgstr "Browse apps"
+
+msgid "Browse commands"
+msgstr "Browse commands"
+
+msgid "Browse shortcuts"
+msgstr "Browse shortcuts"
+
+msgid "Logout"
+msgstr "Logout"
 
 msgid "Search apps"
 msgstr "Search apps"
 
+msgid "Search commands"
+msgstr "Search commands"
+
+msgid "Search shortcuts"
+msgstr "Search shortcuts"
+
+msgid "Search apps, shortcuts, commands"
+msgstr "Search apps, shortcuts, commands"
+
+msgid "Top apps"
+msgstr "Top apps"
+
+msgid "Actions"
+msgstr "Actions"
+
+msgid "All Apps"
+msgstr "All Apps"
+
+msgid "All Commands"
+msgstr "All Commands"
+
+msgid "All Shortcuts"
+msgstr "All Shortcuts"
+
 msgid "DHIS2 {{dhis2Version}}"
 msgstr "DHIS2 {{dhis2Version}}"
 
@@ -62,9 +98,6 @@ msgstr "Help"
 msgid "About DHIS2"
 msgstr "About DHIS2"
 
-msgid "Logout"
-msgstr "Logout"
-
 msgid "New {{appName}} version available"
 msgstr "New {{appName}} version available"
 

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 {
@@ -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 <dialog> 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 {
@@ -67,8 +74,8 @@ describe('Command Palette - List View - Browse Apps View', () => {
                 commands={testCommands}
             />
         )
-        // open modal with (meta + /) keys
-        fireEvent.keyDown(container, { key: '/', metaKey: true })
+        // open modal with (meta + k) keys
+        fireEvent.keyDown(container, { key: 'k', metaKey: true })
 
         // click browse-apps link
         const browseAppsLink = await findByTestId('headerbar-browse-apps')
@@ -87,27 +94,31 @@ describe('Command Palette - List View - Browse Apps View', () => {
         expect(listItems[0].querySelector('span')).toHaveTextContent(
             'Test App 1'
         )
+        listItems[0].focus()
 
         await user.keyboard('{ArrowDown}')
         expect(listItems[0]).not.toHaveClass('highlighted')
         expect(listItems[1]).toHaveClass('highlighted')
         expect(listItems[1].querySelector('span')).toHaveTextContent(
             'Test App 2'
         )
+        listItems[1].focus()
 
         await user.keyboard('{ArrowDown}')
         expect(listItems[1]).not.toHaveClass('highlighted')
         expect(listItems[2]).toHaveClass('highlighted')
         expect(listItems[2].querySelector('span')).toHaveTextContent(
             'Test App 3'
         )
+        listItems[2].focus()
 
         await user.keyboard('{ArrowUp}')
         expect(listItems[2]).not.toHaveClass('highlighted')
         expect(listItems[1]).toHaveClass('highlighted')
         expect(listItems[1].querySelector('span')).toHaveTextContent(
             'Test App 2'
         )
+        listItems[1].focus()
 
         // filter items view
         await user.type(searchField, 'Test App')
@@ -122,10 +133,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'
         )

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 <dialog> 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()
+        // Backspace key goes back to default view
+        await user.keyboard('{Backspace}')
+        expect(queryByText(/All Commands/i)).not.toBeInTheDocument()
         expect(queryByTestId('headerbar-actions-menu')).toBeInTheDocument()
     })
 })

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 <dialog> 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 {

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 (
@@ -53,6 +53,12 @@ export const testShortcuts = [
 ]
 
 describe('Command Palette Component', () => {
+    beforeAll(() => {
+        // Testing environment does not support the <dialog> 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(
@@ -95,52 +101,137 @@ describe('Command Palette Component', () => {
         expect(queryByTestId(modalTest)).not.toBeInTheDocument()
     })
 
-    it('opens and closes Command Palette using ctrl + /', async () => {
+    it('opens and closes Command Palette using ctrl + k', async () => {
         const { container, queryByTestId } = render(
             <CommandPalette apps={[]} shortcuts={[]} commands={[]} />
         )
         // modal not rendered yet
         expect(queryByTestId(modalTest)).not.toBeInTheDocument()
 
-        // open modal with (Ctrl + /) keys
-        fireEvent.keyDown(container, { key: '/', ctrlKey: true })
+        // open modal with (Ctrl + k) keys
+        fireEvent.keyDown(container, { key: 'k', ctrlKey: true })
         expect(queryByTestId(modalTest)).toBeInTheDocument()
 
-        // close modal with (Ctrl + /) keys
-        fireEvent.keyDown(container, { key: '/', ctrlKey: true })
+        // close modal with (Ctrl + k) keys
+        fireEvent.keyDown(container, { key: 'k', ctrlKey: true })
         expect(queryByTestId(modalTest)).not.toBeInTheDocument()
     })
 
-    it('opens and closes Command Palette using meta + /', async () => {
+    it('opens and closes Command Palette using meta + k', async () => {
         const { container, queryByTestId } = render(
             <CommandPalette apps={[]} shortcuts={[]} commands={[]} />
         )
         // modal not rendered yet
         expect(queryByTestId(modalTest)).not.toBeInTheDocument()
 
-        // open modal with (Meta + /) keys
-        fireEvent.keyDown(container, { key: '/', metaKey: true })
+        // open modal with (Meta + k) keys
+        fireEvent.keyDown(container, { key: 'k', metaKey: true })
         expect(queryByTestId(modalTest)).toBeInTheDocument()
 
-        // close modal with (Ctrl + /) keys
-        fireEvent.keyDown(container, { key: '/', metaKey: true })
+        // close modal with (Ctrl + k) keys
+        fireEvent.keyDown(container, { key: 'k', metaKey: true })
         expect(queryByTestId(modalTest)).not.toBeInTheDocument()
     })
 
-    it('closes Command Palette using Esc key', async () => {
+    it('closes Command Palette using Esc key in default view', async () => {
         const user = userEvent.setup()
         const { container, queryByTestId } = render(
             <CommandPalette apps={[]} shortcuts={[]} commands={[]} />
         )
         // modal not rendered yet
         expect(queryByTestId(modalTest)).not.toBeInTheDocument()
 
-        // open modal with (Ctrl + /) keys
-        fireEvent.keyDown(container, { key: '/', ctrlKey: true })
+        // open modal with (Ctrl + k) keys
+        fireEvent.keyDown(container, { key: 'k', ctrlKey: true })
+        expect(queryByTestId(modalTest)).toBeInTheDocument()
+
+        // Esc key closes the modal
+        await user.keyboard('{Escape}')
+        expect(queryByTestId(modalTest)).not.toBeInTheDocument()
+    })
+
+    it('closes Command Palette using Esc key in any list view', async () => {
+        const user = userEvent.setup()
+        const { container, getByTestId, queryByTestId, queryByText } = render(
+            <CommandPalette apps={[]} shortcuts={testShortcuts} commands={[]} />
+        )
+        // modal not rendered yet
+        expect(queryByTestId(modalTest)).not.toBeInTheDocument()
+
+        // open modal with (Ctrl + k) keys
+        fireEvent.keyDown(container, { key: 'k', ctrlKey: true })
         expect(queryByTestId(modalTest)).toBeInTheDocument()
 
+        // go to All Shortcuts (list) view
+        await user.click(getByTestId('headerbar-browse-shortcuts'))
+        expect(queryByText(/Top Apps/i)).not.toBeInTheDocument()
+        expect(queryByText(/All Shortcuts/i)).toBeInTheDocument()
+
         // Esc key closes the modal
         await user.keyboard('{Escape}')
         expect(queryByTestId(modalTest)).not.toBeInTheDocument()
     })
+
+    it('deletes the search filter with the backspace key', async () => {
+        const user = userEvent.setup()
+        const { container, getByPlaceholderText } = render(
+            <CommandPalette
+                apps={testApps}
+                shortcuts={testShortcuts}
+                commands={testCommands}
+            />
+        )
+        // open modal
+        fireEvent.keyDown(container, { key: 'k', ctrlKey: true })
+
+        // Search field
+        const searchField = await getByPlaceholderText(
+            'Search apps, shortcuts, commands'
+        )
+        expect(searchField).toHaveValue('')
+        searchField.focus()
+
+        // backspace
+        await user.type(searchField, '123')
+        expect(searchField).toHaveValue('123')
+
+        await user.keyboard('{Backspace}')
+        expect(searchField).toHaveValue('12')
+
+        await user.keyboard('{Backspace}')
+        expect(searchField).toHaveValue('1')
+
+        await user.keyboard('{Backspace}')
+        expect(searchField).toHaveValue('')
+    })
+
+    it('moves to the default view with backspace key if there is no search filter', async () => {
+        const user = userEvent.setup()
+        const {
+            container,
+            getByPlaceholderText,
+            getByTestId,
+            queryByTestId,
+            queryByText,
+        } = render(
+            <CommandPalette apps={[]} shortcuts={testShortcuts} commands={[]} />
+        )
+        // open modal
+        fireEvent.keyDown(container, { key: 'k', ctrlKey: true })
+
+        // go to All Shortcuts (list) view
+        await user.click(getByTestId('headerbar-browse-shortcuts'))
+
+        expect(queryByTestId('headerbar-actions-menu')).not.toBeInTheDocument()
+        expect(queryByText(/All Shortcuts/i)).toBeInTheDocument()
+
+        // Search field
+        const searchField = await getByPlaceholderText('Search shortcuts')
+        expect(searchField).toHaveValue('')
+
+        // Backspace key goes back to default view
+        await user.keyboard('{Backspace}')
+        expect(queryByText(/All Shortcuts/i)).not.toBeInTheDocument()
+        expect(queryByTestId('headerbar-actions-menu')).toBeInTheDocument()
+    })
 })