diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/Toolbar.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/Toolbar.tsx index 991b92168..74361b6f7 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/Toolbar.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/Toolbar.tsx @@ -6,6 +6,7 @@ import { connect } from 'react-redux'; import { AppState } from '../../../../app.reducer'; import ToolbarButtonUtils, { ToolbarButtonInfo } from '@joplin/lib/services/commands/ToolbarButtonUtils'; import stateToWhenClauseContext from '../../../../services/commands/stateToWhenClauseContext'; +import { _ } from '@joplin/lib/locale'; const { buildStyle } = require('@joplin/lib/theme'); interface ToolbarProps { @@ -29,7 +30,14 @@ const toolbarButtonUtils = new ToolbarButtonUtils(CommandService.instance()); function Toolbar(props: ToolbarProps) { const styles = styles_(props); - return ; + return ( + + ); } const mapStateToProps = (state: AppState) => { diff --git a/packages/app-desktop/gui/NoteToolbar/NoteToolbar.tsx b/packages/app-desktop/gui/NoteToolbar/NoteToolbar.tsx index 07a8f006a..494ead619 100644 --- a/packages/app-desktop/gui/NoteToolbar/NoteToolbar.tsx +++ b/packages/app-desktop/gui/NoteToolbar/NoteToolbar.tsx @@ -6,6 +6,7 @@ import ToolbarButtonUtils, { ToolbarButtonInfo } from '@joplin/lib/services/comm import stateToWhenClauseContext from '../../services/commands/stateToWhenClauseContext'; const { connect } = require('react-redux'); import { buildStyle } from '@joplin/lib/theme'; +import { _ } from '@joplin/lib/locale'; interface NoteToolbarProps { themeId: number; @@ -29,7 +30,14 @@ function styles_(props: NoteToolbarProps) { function NoteToolbar(props: NoteToolbarProps) { const styles = styles_(props); - return ; + return ( + + ); } const toolbarButtonUtils = new ToolbarButtonUtils(CommandService.instance()); diff --git a/packages/app-desktop/gui/ToggleEditorsButton/ToggleEditorsButton.tsx b/packages/app-desktop/gui/ToggleEditorsButton/ToggleEditorsButton.tsx index 946ae7eb1..45bd5347b 100644 --- a/packages/app-desktop/gui/ToggleEditorsButton/ToggleEditorsButton.tsx +++ b/packages/app-desktop/gui/ToggleEditorsButton/ToggleEditorsButton.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import styles_ from './styles'; import { ToolbarButtonInfo } from '@joplin/lib/services/commands/ToolbarButtonUtils'; +import { _ } from '@joplin/lib/locale'; export enum Value { Markdown = 'markdown', @@ -11,6 +12,8 @@ export interface Props { themeId: number; value: Value; toolbarButtonInfo: ToolbarButtonInfo; + tabIndex?: number; + buttonRef?: React.Ref; } export default function ToggleEditorsButton(props: Props) { @@ -18,14 +21,17 @@ export default function ToggleEditorsButton(props: Props) { return ( ); } diff --git a/packages/app-desktop/gui/ToolbarButton/styles/index.ts b/packages/app-desktop/gui/ToolbarButton/styles/index.ts index ec0f04184..1756304b7 100644 --- a/packages/app-desktop/gui/ToolbarButton/styles/index.ts +++ b/packages/app-desktop/gui/ToolbarButton/styles/index.ts @@ -3,46 +3,15 @@ import { ThemeStyle } from '@joplin/lib/theme'; const styled = require('styled-components').default; const { css } = require('styled-components'); -interface RootProps { - readonly theme: ThemeStyle; - readonly disabled: boolean; - readonly hasTitle: boolean; -} - -export const StyledRoot = styled.a` - opacity: ${(props: RootProps) => props.disabled ? 0.3 : 1}; - height: ${(props: RootProps) => props.theme.toolbarHeight}px; - min-height: ${(props: RootProps) => props.theme.toolbarHeight}px; - width: ${(props: RootProps) => props.hasTitle ? 'auto' : `${props.theme.toolbarHeight}px`}; - max-width: ${(props: RootProps) => props.hasTitle ? 'auto' : `${props.theme.toolbarHeight}px`}; - display: flex; - align-items: center; - justify-content: center; - cursor: default; - border-radius: 3px; - box-sizing: border-box; - color: ${(props: RootProps) => props.theme.color3}; - font-size: ${(props: RootProps) => props.theme.toolbarIconSize * 0.8}px; - padding-left: 5px; - padding-right: 5px; - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - - &:hover { - background-color: ${(props: RootProps) => props.disabled ? 'none' : props.theme.backgroundColorHover3}; - } -`; - interface IconProps { readonly theme: ThemeStyle; - readonly title: string; + readonly hasTitle: boolean; } const iconStyle = css` font-size: ${(props: IconProps) => props.theme.toolbarIconSize}px; color: ${(props: IconProps) => props.theme.color3}; - margin-right: ${(props: IconProps) => props.title ? 5 : 0}px; + margin-right: ${(props: IconProps) => props.hasTitle ? 5 : 0}px; pointer-events: none; /* Need this to get button tooltip to work */ `; diff --git a/packages/app-desktop/gui/styles/editor-toolbar.scss b/packages/app-desktop/gui/styles/editor-toolbar.scss new file mode 100644 index 000000000..034feb4d3 --- /dev/null +++ b/packages/app-desktop/gui/styles/editor-toolbar.scss @@ -0,0 +1,21 @@ + +.editor-toolbar { + display: flex; + flex-direction: row; + box-sizing: border-box; + background-color: var(--joplin-background-color3); + padding: var(--joplin-toolbar-padding); + padding-right: var(--joplin-main-padding); + + > .group { + display: flex; + flex-direction: row; + box-sizing: border-box; + min-width: 0; + + &.-right { + flex: 1; + justify-content: flex-end; + } + } +} diff --git a/packages/app-desktop/gui/styles/index.scss b/packages/app-desktop/gui/styles/index.scss index 75455d811..6c5eb1d2a 100644 --- a/packages/app-desktop/gui/styles/index.scss +++ b/packages/app-desktop/gui/styles/index.scss @@ -1,4 +1,6 @@ @use './dialog-modal-layer.scss'; @use './user-webview-dialog.scss'; -@use './prompt-dialog.scss'; \ No newline at end of file +@use './prompt-dialog.scss'; +@use './toolbar-button.scss'; +@use './editor-toolbar.scss'; diff --git a/packages/app-desktop/gui/styles/toolbar-button.scss b/packages/app-desktop/gui/styles/toolbar-button.scss new file mode 100644 index 000000000..7e0b2d458 --- /dev/null +++ b/packages/app-desktop/gui/styles/toolbar-button.scss @@ -0,0 +1,42 @@ + +.toolbar-button { + border: none; + background: transparent; + color: inherit; + padding: 0; + opacity: 1; + height: var(--joplin-toolbar-height); + min-height: var(--joplin-toolbar-height); + width: var(--joplin-toolbar-height); + max-width: var(--joplin-toolbar-height); + + &.-has-title { + width: auto; + max-width: unset; + } + + &:disabled { + opacity: 0.3; + } + + display: flex; + align-items: center; + justify-content: center; + cursor: default; + border-radius: 3px; + box-sizing: border-box; + color: var(--joplin-color3); + font-size: calc(var(--joplin-toolbar-icon-size) * 0.8); + padding-left: 5px; + padding-right: 5px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + + + &:not(:disabled) { + &:hover, &:focus-visible { + background-color: var(--joplin-background-color-hover3); + } + } +} \ No newline at end of file diff --git a/packages/app-desktop/integration-tests/markdownEditor.spec.ts b/packages/app-desktop/integration-tests/markdownEditor.spec.ts index fdca08b42..02fb983ae 100644 --- a/packages/app-desktop/integration-tests/markdownEditor.spec.ts +++ b/packages/app-desktop/integration-tests/markdownEditor.spec.ts @@ -26,5 +26,42 @@ test.describe('markdownEditor', () => { await expect(image).toBeAttached(); await expect(await getImageSourceSize(image)).toMatchObject([117, 30]); }); + + test('arrow keys should navigate the toolbar', async ({ mainWindow }) => { + const mainScreen = new MainScreen(mainWindow); + await mainScreen.waitFor(); + + await mainScreen.createNewNote('Note 1'); + await mainScreen.createNewNote('Note 2'); + const noteEditor = mainScreen.noteEditor; + await noteEditor.focusCodeMirrorEditor(); + + // Escape, then Shift+Tab should focus the toolbar + await mainWindow.keyboard.press('Escape'); + await mainWindow.keyboard.press('Shift+Tab'); + + // Should focus the first item by default, the "back" arrow (back to "Note 1") + const firstItemLocator = noteEditor.toolbarButtonLocator('Back'); + await expect(firstItemLocator).toBeFocused(); + + // Left arrow should wrap to the end + await mainWindow.keyboard.press('ArrowLeft'); + const lastItemLocator = noteEditor.toolbarButtonLocator('Toggle editors'); + await expect(lastItemLocator).toBeFocused(); + + await mainWindow.keyboard.press('ArrowRight'); + await expect(firstItemLocator).toBeFocused(); + + // ArrowRight should skip disabled items (Forward). + await mainWindow.keyboard.press('ArrowRight'); + await expect(noteEditor.toolbarButtonLocator('Toggle external editing')).toBeFocused(); + + // Home/end should navigate to the first/last items + await mainWindow.keyboard.press('End'); + await expect(lastItemLocator).toBeFocused(); + + await mainWindow.keyboard.press('Home'); + await expect(firstItemLocator).toBeFocused(); + }); }); diff --git a/packages/app-desktop/integration-tests/models/NoteEditorScreen.ts b/packages/app-desktop/integration-tests/models/NoteEditorScreen.ts index fca779162..eaff679d3 100644 --- a/packages/app-desktop/integration-tests/models/NoteEditorScreen.ts +++ b/packages/app-desktop/integration-tests/models/NoteEditorScreen.ts @@ -14,8 +14,12 @@ export default class NoteEditorPage { this.codeMirrorEditor = this.containerLocator.locator('.cm-editor'); this.richTextEditor = this.containerLocator.locator('iframe[title="Rich Text Area"]'); this.noteTitleInput = this.containerLocator.locator('.title-input'); - this.attachFileButton = this.containerLocator.locator('[title^="Attach file"]'); - this.toggleEditorsButton = this.containerLocator.locator('[title^="Toggle editors"]'); + this.attachFileButton = this.containerLocator.getByRole('button', { name: 'Attach file' }); + this.toggleEditorsButton = this.containerLocator.getByRole('button', { name: 'Toggle editors' }); + } + + public toolbarButtonLocator(title: string) { + return this.containerLocator.getByRole('button', { name: title }); } public getNoteViewerIframe() { diff --git a/packages/app-desktop/integration-tests/settings.spec.ts b/packages/app-desktop/integration-tests/settings.spec.ts index 9b6d44e81..adba65323 100644 --- a/packages/app-desktop/integration-tests/settings.spec.ts +++ b/packages/app-desktop/integration-tests/settings.spec.ts @@ -8,7 +8,7 @@ test.describe('settings', () => { await mainScreen.waitFor(); // Sort order buttons should be visible by default - const sortOrderLocator = mainScreen.noteListContainer.locator('[title^="Toggle sort order"]'); + const sortOrderLocator = mainScreen.noteListContainer.getByRole('button', { name: 'Toggle sort order' }); await expect(sortOrderLocator).toBeVisible(); await mainScreen.openSettings(electronApp);