Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: command palette navigation #1661

Merged
merged 22 commits into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a49e673
fix: separate the dom and modal container keydown logic
d-rita Jan 25, 2025
1ec35f9
fix: pass interactive handlers to modal container
d-rita Feb 3, 2025
5ad6734
refactor: modal component and logic
d-rita Feb 3, 2025
eb92663
refactor: navigation hook logic
d-rita Feb 4, 2025
801d3c7
test: mock dialog component for tests
d-rita Feb 4, 2025
d3114bb
refactor: modal and search component fixes
d-rita Feb 4, 2025
e2c13cd
test: fix test when user clicks outside the modal
d-rita Feb 4, 2025
478f722
fix: remove focus by hovering
d-rita Feb 4, 2025
8cc2306
fix: consistent highlight of first item and focus on search input
d-rita Feb 4, 2025
bb18ba0
refactor: navigation of home view actions
d-rita Feb 4, 2025
f8e366f
feat: extract home navigation logic into separate hook
d-rita Feb 5, 2025
f387812
fix: rename grid function params
d-rita Feb 5, 2025
4a09596
feat: add logout command
d-rita Feb 5, 2025
8a0e20f
refactor: convert grid, home and list nav hooks to functions
d-rita Feb 6, 2025
e6c0409
feat: add type property to all items in the command palette
d-rita Feb 6, 2025
e89538f
fix: simplify home nav function
d-rita Feb 7, 2025
2303fe3
feat: only show the browse-commands action for multiple available com…
d-rita Feb 10, 2025
f92a414
feat: expose searchable logout action
d-rita Feb 10, 2025
42f138c
fix: change command palette shortcut trigger
cooper-joe Feb 13, 2025
b6415dc
fix: grid item margin reduction
cooper-joe Feb 13, 2025
25beae5
feat: update Esc, Tab and Backspace keys navigation (#1666)
d-rita Feb 14, 2025
182e0c7
fix: remove debug comment
d-rita Feb 19, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions collections/forms/i18n/en.pot
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
43 changes: 38 additions & 5 deletions components/header-bar/i18n/en.pot
Original file line number Diff line number Diff line change
Expand Up @@ -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}}"

Expand Down Expand Up @@ -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"

Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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'
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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()
})
})
Loading
Loading