From fa99c6db337236318589d6f33be420c282c7aed9 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Thu, 23 Jan 2025 10:54:17 -0800 Subject: [PATCH 01/19] Desktop: Accessibility: Fix missing button labels in "change app layout" screen --- .../app-desktop/gui/ResizableLayout/MoveButtons.tsx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx index 26905c67fd6..686116ef1ba 100644 --- a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx +++ b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx @@ -3,6 +3,7 @@ import { useCallback } from 'react'; import Button, { ButtonLevel } from '../Button/Button'; import { MoveDirection } from './utils/movements'; import styled from 'styled-components'; +import { _ } from '@joplin/lib/locale'; const StyledRoot = styled.div` display: flex; @@ -53,11 +54,21 @@ export default function MoveButtons(props: Props) { throw new Error('Unreachable'); } + const iconLabel = (dir: MoveDirection) => { + if (dir === MoveDirection.Up) return _('Move up'); + if (dir === MoveDirection.Down) return _('Move down'); + if (dir === MoveDirection.Left) return _('Move left'); + if (dir === MoveDirection.Right) return _('Move right'); + const unreachable: never = dir; + throw new Error(`Invalid direction: ${unreachable}`); + }; + function renderButton(dir: MoveDirection) { return onButtonClick(dir)} />; } @@ -69,7 +80,7 @@ export default function MoveButtons(props: Props) { {renderButton(MoveDirection.Left)} - + {renderButton(MoveDirection.Right)} From b9d69a54755ba0d7ea587ee9e238d6e6c778d527 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Thu, 23 Jan 2025 11:44:33 -0800 Subject: [PATCH 02/19] Types --- .../gui/ResizableLayout/ResizableLayout.tsx | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx index 79d1d7d2688..c24b84c47a1 100644 --- a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx +++ b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx @@ -8,7 +8,7 @@ import { Size, LayoutItem } from './utils/types'; import { canMove, MoveDirection } from './utils/movements'; import MoveButtons, { MoveButtonClickEvent } from './MoveButtons'; import { StyledWrapperRoot, StyledMoveOverlay, MoveModeRootWrapper, MoveModeRootMessage } from './utils/style'; -import { Resizable } from 're-resizable'; +import { Resizable, ResizeCallback, ResizeStartCallback } from 're-resizable'; const EventEmitter = require('events'); interface OnResizeEvent { @@ -34,7 +34,7 @@ function itemVisible(item: LayoutItem, moveMode: boolean) { } // eslint-disable-next-line @typescript-eslint/ban-types, @typescript-eslint/no-explicit-any -- Old code before rule was applied, Old code before rule was applied -function renderContainer(item: LayoutItem, parent: LayoutItem | null, sizes: LayoutItemSizes, resizedItemMaxSize: Size | null, onResizeStart: Function, onResize: Function, onResizeStop: Function, children: any[], isLastChild: boolean, moveMode: boolean): any { +function renderContainer(item: LayoutItem, parent: LayoutItem | null, sizes: LayoutItemSizes, resizedItemMaxSize: Size | null, onResizeStart: ResizeStartCallback, onResize: ResizeCallback, onResizeStop: ResizeCallback, children: any[], isLastChild: boolean, moveMode: boolean): any { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied const style: any = { display: itemVisible(item, moveMode) ? 'flex' : 'none', @@ -62,12 +62,9 @@ function renderContainer(item: LayoutItem, parent: LayoutItem | null, sizes: Lay className={className} style={style} size={size} - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - onResizeStart={onResizeStart as any} - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - onResize={onResize as any} - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - onResizeStop={onResizeStop as any} + onResizeStart={onResizeStart} + onResize={onResize} + onResizeStop={onResizeStop} enable={enable} minWidth={'minWidth' in item ? item.minWidth : itemMinWidth} minHeight={'minHeight' in item ? item.minHeight : itemMinHeight} From 00a19dee32abb83daa4761630a93f9e17fc1da2c Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Thu, 23 Jan 2025 12:20:00 -0800 Subject: [PATCH 03/19] Show move controls in a dialog --- .../gui/ResizableLayout/ResizableLayout.tsx | 51 ++++++++++--------- .../gui/styles/dialog-modal-layer.scss | 7 ++- packages/app-desktop/gui/styles/index.scss | 1 + .../styles/resizable-panels-move-dialog.scss | 7 +++ 4 files changed, 40 insertions(+), 26 deletions(-) create mode 100644 packages/app-desktop/gui/styles/resizable-panels-move-dialog.scss diff --git a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx index c24b84c47a1..be073728eea 100644 --- a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx +++ b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx @@ -7,8 +7,9 @@ import validateLayout from './utils/validateLayout'; import { Size, LayoutItem } from './utils/types'; import { canMove, MoveDirection } from './utils/movements'; import MoveButtons, { MoveButtonClickEvent } from './MoveButtons'; -import { StyledWrapperRoot, StyledMoveOverlay, MoveModeRootWrapper, MoveModeRootMessage } from './utils/style'; +import { StyledWrapperRoot, StyledMoveOverlay, MoveModeRootMessage } from './utils/style'; import { Resizable, ResizeCallback, ResizeStartCallback } from 're-resizable'; +import Dialog from '../Dialog'; const EventEmitter = require('events'); interface OnResizeEvent { @@ -33,8 +34,7 @@ function itemVisible(item: LayoutItem, moveMode: boolean) { return item.visible !== false; } -// eslint-disable-next-line @typescript-eslint/ban-types, @typescript-eslint/no-explicit-any -- Old code before rule was applied, Old code before rule was applied -function renderContainer(item: LayoutItem, parent: LayoutItem | null, sizes: LayoutItemSizes, resizedItemMaxSize: Size | null, onResizeStart: ResizeStartCallback, onResize: ResizeCallback, onResizeStop: ResizeCallback, children: any[], isLastChild: boolean, moveMode: boolean): any { +function renderContainer(item: LayoutItem, parent: LayoutItem | null, sizes: LayoutItemSizes, resizedItemMaxSize: Size | null, onResizeStart: ResizeStartCallback, onResize: ResizeCallback, onResizeStop: ResizeCallback, children: React.ReactNode[], isLastChild: boolean, moveMode: boolean) { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied const style: any = { display: itemVisible(item, moveMode) ? 'flex' : 'none', @@ -89,9 +89,8 @@ function ResizableLayout(props: Props) { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied const [resizedItem, setResizedItem] = useState(null); - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - function renderItemWrapper(comp: any, item: LayoutItem, parent: LayoutItem | null, size: Size, moveMode: boolean) { - const moveOverlay = moveMode ? ( + function renderItemWrapper(comp: React.ReactNode, item: LayoutItem, parent: LayoutItem | null, size: Size, showMoveControls: boolean) { + const moveControls = ( - ) : null; + ); return ( - {moveOverlay} - {comp} + {showMoveControls ? moveControls : comp} ); } - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - function renderLayoutItem(item: LayoutItem, parent: LayoutItem | null, sizes: LayoutItemSizes, isVisible: boolean, isLastChild: boolean): any { + function renderLayoutItem( + item: LayoutItem, parent: LayoutItem | null, sizes: LayoutItemSizes, isVisible: boolean, isLastChild: boolean, showMoveControls: boolean, + ): React.ReactNode { function onResizeStart() { setResizedItem({ key: item.key, @@ -163,14 +162,16 @@ function ResizableLayout(props: Props) { visible: isVisible, }); - const wrapper = renderItemWrapper(comp, item, parent, size, props.moveMode); + const wrapper = renderItemWrapper(comp, item, parent, size, showMoveControls); return renderContainer(item, parent, sizes, resizedItemMaxSize, onResizeStart, onResize, onResizeStop, [wrapper], isLastChild, props.moveMode); } else { const childrenComponents = []; for (let i = 0; i < item.children.length; i++) { const child = item.children[i]; - childrenComponents.push(renderLayoutItem(child, item, sizes, isVisible && itemVisible(child, props.moveMode), i === item.children.length - 1)); + childrenComponents.push( + renderLayoutItem(child, item, sizes, isVisible && itemVisible(child, props.moveMode), i === item.children.length - 1, showMoveControls), + ); } return renderContainer(item, parent, sizes, resizedItemMaxSize, onResizeStart, onResize, onResizeStop, childrenComponents, isLastChild, props.moveMode); @@ -184,22 +185,24 @@ function ResizableLayout(props: Props) { useWindowResizeEvent(eventEmitter); const sizes = useLayoutItemSizes(props.layout, props.moveMode); - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - function renderMoveModeBox(rootComp: any) { - return ( - + const renderRoot = (moveControlsOnly: boolean) => { + return renderLayoutItem(props.layout, null, sizes, itemVisible(props.layout, props.moveMode), true, moveControlsOnly); + }; + + function renderMoveModeBox() { + return
+ {props.moveModeMessage} - {rootComp} - - ); + {renderRoot(true)} + + {renderRoot(false)} +
; } - const rootComp = renderLayoutItem(props.layout, null, sizes, itemVisible(props.layout, props.moveMode), true); - if (props.moveMode) { - return renderMoveModeBox(rootComp); + return renderMoveModeBox(); } else { - return rootComp; + return renderRoot(false); } } diff --git a/packages/app-desktop/gui/styles/dialog-modal-layer.scss b/packages/app-desktop/gui/styles/dialog-modal-layer.scss index d8c81838cd3..9b5bdd8a894 100644 --- a/packages/app-desktop/gui/styles/dialog-modal-layer.scss +++ b/packages/app-desktop/gui/styles/dialog-modal-layer.scss @@ -32,13 +32,16 @@ } &.-fullscreen { + max-width: 100vw; + max-height: 100vh; + &::backdrop { background-color: var(--joplin-background-color); } > .content { - width: calc(100% - 20px); - padding: 10px; + margin: 0; + padding: 0; border-radius: 0; box-shadow: none; background-color: transparent; diff --git a/packages/app-desktop/gui/styles/index.scss b/packages/app-desktop/gui/styles/index.scss index 0217913a6bb..97b49e00381 100644 --- a/packages/app-desktop/gui/styles/index.scss +++ b/packages/app-desktop/gui/styles/index.scss @@ -11,3 +11,4 @@ @use './dialog-anchor-node.scss'; @use './note-editor-wrapper.scss'; @use './text-input.scss'; +@use './resizable-panels-move-dialog.scss'; diff --git a/packages/app-desktop/gui/styles/resizable-panels-move-dialog.scss b/packages/app-desktop/gui/styles/resizable-panels-move-dialog.scss new file mode 100644 index 00000000000..ff4f81a2330 --- /dev/null +++ b/packages/app-desktop/gui/styles/resizable-panels-move-dialog.scss @@ -0,0 +1,7 @@ +.resizable-panels-move-dialog { + padding: 0; + + &::backdrop { + background-color: transparent !important; + } +} \ No newline at end of file From 15c51f44caf8e5b1b544f826fcb0802e15b92053 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Thu, 23 Jan 2025 12:45:34 -0800 Subject: [PATCH 04/19] Fix move mode header styles --- .../app-desktop/gui/ResizableLayout/utils/style.ts | 12 +++--------- .../gui/styles/resizable-panels-move-dialog.scss | 7 +++++++ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/app-desktop/gui/ResizableLayout/utils/style.ts b/packages/app-desktop/gui/ResizableLayout/utils/style.ts index 5637cbd6ee8..bfb610480a8 100644 --- a/packages/app-desktop/gui/ResizableLayout/utils/style.ts +++ b/packages/app-desktop/gui/ResizableLayout/utils/style.ts @@ -28,18 +28,12 @@ export const StyledMoveOverlay = styled.div` height: 100%; `; -export const MoveModeRootWrapper = styled.div` - position:relative; - display: flex; - align-items: center; - justify-content: center; -`; - -export const MoveModeRootMessage = styled.div` +export const MoveModeRootMessage = styled.h1` position:absolute; bottom: 10px; + z-index:200; background-color: ${props => props.theme.backgroundColor}; padding: 10px; - border-radius: 5; + border-radius: 5px; `; diff --git a/packages/app-desktop/gui/styles/resizable-panels-move-dialog.scss b/packages/app-desktop/gui/styles/resizable-panels-move-dialog.scss index ff4f81a2330..3685607e056 100644 --- a/packages/app-desktop/gui/styles/resizable-panels-move-dialog.scss +++ b/packages/app-desktop/gui/styles/resizable-panels-move-dialog.scss @@ -1,6 +1,13 @@ .resizable-panels-move-dialog { padding: 0; + > .content { + position:relative; + display: flex; + align-items: center; + justify-content: center; + } + &::backdrop { background-color: transparent !important; } From 54b7ce60947648ea2f373510121aa167e384422f Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Thu, 23 Jan 2025 13:18:45 -0800 Subject: [PATCH 05/19] Add accessible labels to the move buttons (describe what they move) --- packages/app-desktop/gui/MainScreen.tsx | 18 +++++++++ .../gui/ResizableLayout/MoveButtons.tsx | 7 +++- .../gui/ResizableLayout/ResizableLayout.tsx | 39 +++++++++++-------- .../lib/services/plugins/PluginService.ts | 8 ++++ 4 files changed, 54 insertions(+), 18 deletions(-) diff --git a/packages/app-desktop/gui/MainScreen.tsx b/packages/app-desktop/gui/MainScreen.tsx index 195c689e9c5..63a0b35a7d3 100644 --- a/packages/app-desktop/gui/MainScreen.tsx +++ b/packages/app-desktop/gui/MainScreen.tsx @@ -43,6 +43,7 @@ import UpdateNotification from './UpdateNotification/UpdateNotification'; import NoteEditor from './NoteEditor/NoteEditor'; import PluginNotification from './PluginNotification/PluginNotification'; import { Toast } from '@joplin/lib/services/plugins/api/types'; +import PluginService from '@joplin/lib/services/plugins/PluginService'; const ipcRenderer = require('electron').ipcRenderer; @@ -121,6 +122,18 @@ const defaultLayout: LayoutItem = { ], }; +const layoutKeyToLabel = (key: string, plugins: PluginStates) => { + if (key === 'sideBar') return _('Sidebar'); + if (key === 'noteList') return _('Note list'); + if (key === 'editor') return _('Editor'); + + const viewInfo = pluginUtils.viewInfoByViewId(plugins, key); + if (viewInfo) { + return PluginService.instance().safePluginNameById(viewInfo.plugin.id); + } + return key; +}; + class MainScreenComponent extends React.Component { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied @@ -728,6 +741,10 @@ class MainScreenComponent extends React.Component { ); } + private layoutKeyToLabel = (key: string) => { + return layoutKeyToLabel(key, this.props.plugins); + }; + public render() { const theme = themeStyle(this.props.themeId); const style = { @@ -746,6 +763,7 @@ class MainScreenComponent extends React.Component { onResize={this.resizableLayout_resize} onMoveButtonClick={this.resizableLayout_moveButtonClick} renderItem={this.resizableLayout_renderItem} + layoutKeyToLabel={this.layoutKeyToLabel} moveMode={this.props.layoutMoveMode} moveModeMessage={_('Use the arrows to move the layout items. Press "Escape" to exit.')} /> diff --git a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx index 686116ef1ba..32951cd714e 100644 --- a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx +++ b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { useCallback } from 'react'; +import { useCallback, useId } from 'react'; import Button, { ButtonLevel } from '../Button/Button'; import { MoveDirection } from './utils/movements'; import styled from 'styled-components'; @@ -35,6 +35,7 @@ export interface MoveButtonClickEvent { interface Props { onClick(event: MoveButtonClickEvent): void; itemKey: string; + itemLabel: string; canMoveLeft: boolean; canMoveRight: boolean; canMoveUp: boolean; @@ -63,12 +64,15 @@ export default function MoveButtons(props: Props) { throw new Error(`Invalid direction: ${unreachable}`); }; + const descriptionId = useId(); + function renderButton(dir: MoveDirection) { return onButtonClick(dir)} />; } @@ -86,6 +90,7 @@ export default function MoveButtons(props: Props) { {renderButton(MoveDirection.Down)} + {props.itemLabel} ); } diff --git a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx index be073728eea..40e6df44b2e 100644 --- a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx +++ b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx @@ -18,6 +18,7 @@ interface OnResizeEvent { interface Props { layout: LayoutItem; + layoutKeyToLabel: (key: string)=> string; onResize(event: OnResizeEvent): void; width?: number; height?: number; @@ -89,29 +90,34 @@ function ResizableLayout(props: Props) { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied const [resizedItem, setResizedItem] = useState(null); - function renderItemWrapper(comp: React.ReactNode, item: LayoutItem, parent: LayoutItem | null, size: Size, showMoveControls: boolean) { - const moveControls = ( - - - + const renderMoveControls = (item: LayoutItem, parent: LayoutItem | null, size: Size) => { + return ( + + + + + ); + }; + function renderItemWrapper(comp: React.ReactNode, item: LayoutItem, size: Size) { return ( - {showMoveControls ? moveControls : comp} + {comp} ); } function renderLayoutItem( - item: LayoutItem, parent: LayoutItem | null, sizes: LayoutItemSizes, isVisible: boolean, isLastChild: boolean, showMoveControls: boolean, + item: LayoutItem, parent: LayoutItem | null, sizes: LayoutItemSizes, isVisible: boolean, isLastChild: boolean, onlyMoveControls: boolean, ): React.ReactNode { function onResizeStart() { setResizedItem({ @@ -162,15 +168,14 @@ function ResizableLayout(props: Props) { visible: isVisible, }); - const wrapper = renderItemWrapper(comp, item, parent, size, showMoveControls); - + const wrapper = onlyMoveControls ? renderMoveControls(item, parent, size) : renderItemWrapper(comp, item, size); return renderContainer(item, parent, sizes, resizedItemMaxSize, onResizeStart, onResize, onResizeStop, [wrapper], isLastChild, props.moveMode); } else { const childrenComponents = []; for (let i = 0; i < item.children.length; i++) { const child = item.children[i]; childrenComponents.push( - renderLayoutItem(child, item, sizes, isVisible && itemVisible(child, props.moveMode), i === item.children.length - 1, showMoveControls), + renderLayoutItem(child, item, sizes, isVisible && itemVisible(child, props.moveMode), i === item.children.length - 1, onlyMoveControls), ); } diff --git a/packages/lib/services/plugins/PluginService.ts b/packages/lib/services/plugins/PluginService.ts index 1b9338279a7..d2cf81f1113 100644 --- a/packages/lib/services/plugins/PluginService.ts +++ b/packages/lib/services/plugins/PluginService.ts @@ -203,6 +203,14 @@ export default class PluginService extends BaseService { return this.plugins_[id]; } + public safePluginNameById(id: string) { + if (!this.plugins_[id]) { + return id; + } + + return this.pluginById(id).manifest?.name ?? 'Unknown'; + } + public viewControllerByViewId(id: string): ViewController|null { for (const [, plugin] of Object.entries(this.plugins_)) { if (plugin.hasViewController(id)) return plugin.viewController(id); From 44306d05a435c980fb5b2fe24514d683d4ff5f33 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Thu, 23 Jan 2025 13:43:07 -0800 Subject: [PATCH 06/19] Improve move button label styling --- .../app-desktop/gui/ResizableLayout/MoveButtons.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx index 32951cd714e..bed365e19f8 100644 --- a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx +++ b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx @@ -11,6 +11,15 @@ const StyledRoot = styled.div` padding: 5px; background-color: ${props => props.theme.backgroundColor}; border-radius: 5px; + + > .label { + text-align: center; + text-overflow: ellipsis; + white-space: nowrap; + + font-weight: bold; + padding-top: 4px; + } `; const ButtonRow = styled.div` @@ -90,7 +99,7 @@ export default function MoveButtons(props: Props) { {renderButton(MoveDirection.Down)} - {props.itemLabel} +
{props.itemLabel}
); } From 92c4127caa48a60a22073f2e9a01c5d4fe47dca7 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Thu, 23 Jan 2025 14:25:16 -0800 Subject: [PATCH 07/19] Fix button loses focus after clicking --- packages/app-desktop/gui/Button/Button.tsx | 2 ++ .../app-desktop/gui/ResizableLayout/MoveButtons.tsx | 10 +++++++++- .../gui/ResizableLayout/ResizableLayout.tsx | 11 +++++++++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/app-desktop/gui/Button/Button.tsx b/packages/app-desktop/gui/Button/Button.tsx index 58f4f7ec1b9..33e7bd9ce68 100644 --- a/packages/app-desktop/gui/Button/Button.tsx +++ b/packages/app-desktop/gui/Button/Button.tsx @@ -30,6 +30,7 @@ interface Props { iconAnimation?: string; tooltip?: string; disabled?: boolean; + autoFocus?: boolean; // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied; style?: any; size?: ButtonSize; @@ -253,6 +254,7 @@ const Button = React.forwardRef((props: Props, ref: any) => { size={props.size} style={props.style} disabled={props.disabled} + autoFocus={props.autoFocus} title={props.tooltip} className={props.className} iconOnly={iconOnly} diff --git a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx index bed365e19f8..1c56e024673 100644 --- a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx +++ b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx @@ -36,9 +36,12 @@ const ArrowButton = styled(Button)` opacity: ${props => props.disabled ? 0.2 : 1}; `; +type ButtonKey = string; + export interface MoveButtonClickEvent { direction: MoveDirection; itemKey: string; + buttonKey: ButtonKey; } interface Props { @@ -49,11 +52,14 @@ interface Props { canMoveRight: boolean; canMoveUp: boolean; canMoveDown: boolean; + + // A buttonKey to auto-focus + autoFocusKey: ButtonKey|null; } export default function MoveButtons(props: Props) { const onButtonClick = useCallback((direction: MoveDirection) => { - props.onClick({ direction, itemKey: props.itemKey }); + props.onClick({ direction, itemKey: props.itemKey, buttonKey: `${props.itemKey}-${direction}` }); }, [props.onClick, props.itemKey]); function canMove(dir: MoveDirection) { @@ -76,12 +82,14 @@ export default function MoveButtons(props: Props) { const descriptionId = useId(); function renderButton(dir: MoveDirection) { + const key = `${props.itemKey}-${dir}`; return onButtonClick(dir)} />; } diff --git a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx index 40e6df44b2e..4a26b79c6c2 100644 --- a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx +++ b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { useRef, useState, useEffect } from 'react'; +import { useRef, useState, useEffect, useCallback } from 'react'; import useWindowResizeEvent from './utils/useWindowResizeEvent'; import setLayoutItemProps from './utils/setLayoutItemProps'; import useLayoutItemSizes, { LayoutItemSizes, itemSize, calculateMaxSizeAvailableForItem, itemMinWidth, itemMinHeight } from './utils/useLayoutItemSizes'; @@ -89,15 +89,22 @@ function ResizableLayout(props: Props) { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied const [resizedItem, setResizedItem] = useState(null); + const lastUsedMoveButtonKey = useRef(null); + + const onMoveButtonClick = useCallback((event: MoveButtonClickEvent) => { + lastUsedMoveButtonKey.current = event.buttonKey; + props.onMoveButtonClick(event); + }, [props.onMoveButtonClick]); const renderMoveControls = (item: LayoutItem, parent: LayoutItem | null, size: Size) => { return ( Date: Thu, 23 Jan 2025 16:51:53 -0800 Subject: [PATCH 08/19] Fix refocus when a button becomes disabled after clicking --- .../gui/ResizableLayout/MoveButtons.tsx | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx index 1c56e024673..6d4b36c6e83 100644 --- a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx +++ b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx @@ -53,7 +53,8 @@ interface Props { canMoveUp: boolean; canMoveDown: boolean; - // A buttonKey to auto-focus + // A buttonKey to auto-focus. As one of the move buttons can change the app's layout, + // it's important to refocus the last-clicked button. autoFocusKey: ButtonKey|null; } @@ -81,15 +82,36 @@ export default function MoveButtons(props: Props) { const descriptionId = useId(); + const buttonKey = (dir: MoveDirection) => `${props.itemKey}-${dir}`; + const autoFocusDirection = (() => { + if (!props.autoFocusKey) return undefined; + + const buttonDirections = [MoveDirection.Up, MoveDirection.Down, MoveDirection.Left, MoveDirection.Right]; + const autoFocusDirection = buttonDirections.find( + direction => buttonKey(direction) === props.autoFocusKey, + ); + + if (!autoFocusDirection) { + return null; + } + + const autoFocusDirectionEnabled = autoFocusDirection && canMove(autoFocusDirection); + if (autoFocusDirectionEnabled) { + return autoFocusDirection; + } else { + // Select an enabled direction instead + return buttonDirections.find(dir => canMove(dir)); + } + })(); + function renderButton(dir: MoveDirection) { - const key = `${props.itemKey}-${dir}`; return onButtonClick(dir)} />; } From 1c2c758d07676537dae6c955b9d6ae051b1ed141 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Thu, 23 Jan 2025 17:25:55 -0800 Subject: [PATCH 09/19] Make styles closer to the originals --- packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx | 8 ++------ packages/app-desktop/gui/ResizableLayout/utils/style.ts | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx index 6d4b36c6e83..6ef203b51ed 100644 --- a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx +++ b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx @@ -13,12 +13,8 @@ const StyledRoot = styled.div` border-radius: 5px; > .label { - text-align: center; - text-overflow: ellipsis; - white-space: nowrap; - - font-weight: bold; - padding-top: 4px; + // Used only for accessibility tools + display: none; } `; diff --git a/packages/app-desktop/gui/ResizableLayout/utils/style.ts b/packages/app-desktop/gui/ResizableLayout/utils/style.ts index bfb610480a8..fabc5e83417 100644 --- a/packages/app-desktop/gui/ResizableLayout/utils/style.ts +++ b/packages/app-desktop/gui/ResizableLayout/utils/style.ts @@ -29,11 +29,11 @@ export const StyledMoveOverlay = styled.div` `; export const MoveModeRootMessage = styled.h1` - position:absolute; + position: absolute; bottom: 10px; + font-size: 1em; z-index:200; background-color: ${props => props.theme.backgroundColor}; padding: 10px; - border-radius: 5px; `; From b2c553c11d91eda1b1bb817b9922b6ae2b4b18a3 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Thu, 23 Jan 2025 19:20:49 -0800 Subject: [PATCH 10/19] Attempting to announce the position of the moved item --- packages/app-desktop/gui/Button/Button.tsx | 65 ++++++++----------- packages/app-desktop/gui/MainScreen.tsx | 6 +- .../gui/ResizableLayout/MoveButtons.tsx | 17 +++-- .../gui/ResizableLayout/ResizableLayout.tsx | 32 ++++++--- 4 files changed, 68 insertions(+), 52 deletions(-) diff --git a/packages/app-desktop/gui/Button/Button.tsx b/packages/app-desktop/gui/Button/Button.tsx index 33e7bd9ce68..05bfa826659 100644 --- a/packages/app-desktop/gui/Button/Button.tsx +++ b/packages/app-desktop/gui/Button/Button.tsx @@ -18,29 +18,24 @@ export enum ButtonSize { Normal = 2, } -interface Props { +type BaseButtonProps = Omit< +React.DetailedHTMLProps, HTMLButtonElement>, +'onClick' +>; +interface Props extends BaseButtonProps { title?: string; iconName?: string; level?: ButtonLevel; iconLabel?: string; - className?: string; - // eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied - onClick?: Function; + onClick?: ()=> void; color?: string; iconAnimation?: string; tooltip?: string; disabled?: boolean; - autoFocus?: boolean; - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied; - style?: any; size?: ButtonSize; isSquare?: boolean; iconOnly?: boolean; fontSize?: number; - - 'aria-controls'?: string; - 'aria-describedby'?: string; - 'aria-expanded'?: string; } const StyledTitle = styled.span` @@ -217,56 +212,52 @@ function buttonClass(level: ButtonLevel) { return StyledButtonSecondary; } +const Button = React.forwardRef(({ + iconName, iconLabel, iconAnimation, color, title, level, fontSize, isSquare, tooltip, disabled, onClick: propsOnClick, ...unusedProps // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied; -const Button = React.forwardRef((props: Props, ref: any) => { - const iconOnly = props.iconName && !props.title; +}: Props, ref: any) => { + const iconOnly = iconName && !title; - const StyledButton = buttonClass(props.level); + const StyledButton = buttonClass(level); function renderIcon() { - if (!props.iconName) return null; + if (!iconName) return null; return ; } function renderTitle() { - if (!props.title) return null; - return {props.title}; + if (!title) return null; + return {title}; } function onClick() { - if (props.disabled) return; - props.onClick(); + if (disabled) return; + propsOnClick(); } return ( {renderIcon()} {renderTitle()} diff --git a/packages/app-desktop/gui/MainScreen.tsx b/packages/app-desktop/gui/MainScreen.tsx index 63a0b35a7d3..51f285a1793 100644 --- a/packages/app-desktop/gui/MainScreen.tsx +++ b/packages/app-desktop/gui/MainScreen.tsx @@ -741,8 +741,8 @@ class MainScreenComponent extends React.Component { ); } - private layoutKeyToLabel = (key: string) => { - return layoutKeyToLabel(key, this.props.plugins); + private layoutKeyToLabel = (key: string, row: number, column: number) => { + return `${layoutKeyToLabel(key, this.props.plugins)} (${row}, ${column})`; }; public render() { @@ -763,7 +763,7 @@ class MainScreenComponent extends React.Component { onResize={this.resizableLayout_resize} onMoveButtonClick={this.resizableLayout_moveButtonClick} renderItem={this.resizableLayout_renderItem} - layoutKeyToLabel={this.layoutKeyToLabel} + layoutLabel={this.layoutKeyToLabel} moveMode={this.props.layoutMoveMode} moveModeMessage={_('Use the arrows to move the layout items. Press "Escape" to exit.')} /> diff --git a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx index 6ef203b51ed..2539ef8805a 100644 --- a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx +++ b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx @@ -76,7 +76,8 @@ export default function MoveButtons(props: Props) { throw new Error(`Invalid direction: ${unreachable}`); }; - const descriptionId = useId(); + const baseId = useId(); + const descriptionId = `${baseId}-description`; const buttonKey = (dir: MoveDirection) => `${props.itemKey}-${dir}`; const autoFocusDirection = (() => { @@ -101,13 +102,17 @@ export default function MoveButtons(props: Props) { })(); function renderButton(dir: MoveDirection) { + const buttonId = `${baseId}-button-${dir}`; return onButtonClick(dir)} />; } @@ -125,7 +130,11 @@ export default function MoveButtons(props: Props) { {renderButton(MoveDirection.Down)} -
{props.itemLabel}
+
{props.itemLabel}
); } diff --git a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx index 4a26b79c6c2..32d44c1ef0e 100644 --- a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx +++ b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx @@ -4,7 +4,7 @@ import useWindowResizeEvent from './utils/useWindowResizeEvent'; import setLayoutItemProps from './utils/setLayoutItemProps'; import useLayoutItemSizes, { LayoutItemSizes, itemSize, calculateMaxSizeAvailableForItem, itemMinWidth, itemMinHeight } from './utils/useLayoutItemSizes'; import validateLayout from './utils/validateLayout'; -import { Size, LayoutItem } from './utils/types'; +import { Size, LayoutItem, LayoutItemDirection } from './utils/types'; import { canMove, MoveDirection } from './utils/movements'; import MoveButtons, { MoveButtonClickEvent } from './MoveButtons'; import { StyledWrapperRoot, StyledMoveOverlay, MoveModeRootMessage } from './utils/style'; @@ -18,7 +18,7 @@ interface OnResizeEvent { interface Props { layout: LayoutItem; - layoutKeyToLabel: (key: string)=> string; + layoutLabel: (itemKey: string, rowIndex: number, colIndex: number)=> string; onResize(event: OnResizeEvent): void; width?: number; height?: number; @@ -96,14 +96,19 @@ function ResizableLayout(props: Props) { props.onMoveButtonClick(event); }, [props.onMoveButtonClick]); - const renderMoveControls = (item: LayoutItem, parent: LayoutItem | null, size: Size) => { + type ItemPosition = { + rowIndex: number; + colIndex: number; + }; + + const renderMoveControls = (item: LayoutItem, parent: LayoutItem | null, size: Size, { rowIndex, colIndex }: ItemPosition) => { return ( { - return renderLayoutItem(props.layout, null, sizes, itemVisible(props.layout, props.moveMode), true, moveControlsOnly); + return renderLayoutItem(props.layout, null, sizes, itemVisible(props.layout, props.moveMode), true, moveControlsOnly, { rowIndex: 0, colIndex: 0 }); }; function renderMoveModeBox() { From 550fa20cf6aa841ce314dc80b034b72bae360c6d Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Thu, 23 Jan 2025 19:24:25 -0800 Subject: [PATCH 11/19] Revert attempt to announce position of the just-moved menu --- packages/app-desktop/gui/MainScreen.tsx | 6 ++-- .../gui/ResizableLayout/MoveButtons.tsx | 17 +++------- .../gui/ResizableLayout/ResizableLayout.tsx | 32 +++++-------------- 3 files changed, 15 insertions(+), 40 deletions(-) diff --git a/packages/app-desktop/gui/MainScreen.tsx b/packages/app-desktop/gui/MainScreen.tsx index 51f285a1793..63a0b35a7d3 100644 --- a/packages/app-desktop/gui/MainScreen.tsx +++ b/packages/app-desktop/gui/MainScreen.tsx @@ -741,8 +741,8 @@ class MainScreenComponent extends React.Component { ); } - private layoutKeyToLabel = (key: string, row: number, column: number) => { - return `${layoutKeyToLabel(key, this.props.plugins)} (${row}, ${column})`; + private layoutKeyToLabel = (key: string) => { + return layoutKeyToLabel(key, this.props.plugins); }; public render() { @@ -763,7 +763,7 @@ class MainScreenComponent extends React.Component { onResize={this.resizableLayout_resize} onMoveButtonClick={this.resizableLayout_moveButtonClick} renderItem={this.resizableLayout_renderItem} - layoutLabel={this.layoutKeyToLabel} + layoutKeyToLabel={this.layoutKeyToLabel} moveMode={this.props.layoutMoveMode} moveModeMessage={_('Use the arrows to move the layout items. Press "Escape" to exit.')} /> diff --git a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx index 2539ef8805a..6ef203b51ed 100644 --- a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx +++ b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx @@ -76,8 +76,7 @@ export default function MoveButtons(props: Props) { throw new Error(`Invalid direction: ${unreachable}`); }; - const baseId = useId(); - const descriptionId = `${baseId}-description`; + const descriptionId = useId(); const buttonKey = (dir: MoveDirection) => `${props.itemKey}-${dir}`; const autoFocusDirection = (() => { @@ -102,17 +101,13 @@ export default function MoveButtons(props: Props) { })(); function renderButton(dir: MoveDirection) { - const buttonId = `${baseId}-button-${dir}`; return onButtonClick(dir)} />; } @@ -130,11 +125,7 @@ export default function MoveButtons(props: Props) { {renderButton(MoveDirection.Down)} -
{props.itemLabel}
+
{props.itemLabel}
); } diff --git a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx index 32d44c1ef0e..4a26b79c6c2 100644 --- a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx +++ b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx @@ -4,7 +4,7 @@ import useWindowResizeEvent from './utils/useWindowResizeEvent'; import setLayoutItemProps from './utils/setLayoutItemProps'; import useLayoutItemSizes, { LayoutItemSizes, itemSize, calculateMaxSizeAvailableForItem, itemMinWidth, itemMinHeight } from './utils/useLayoutItemSizes'; import validateLayout from './utils/validateLayout'; -import { Size, LayoutItem, LayoutItemDirection } from './utils/types'; +import { Size, LayoutItem } from './utils/types'; import { canMove, MoveDirection } from './utils/movements'; import MoveButtons, { MoveButtonClickEvent } from './MoveButtons'; import { StyledWrapperRoot, StyledMoveOverlay, MoveModeRootMessage } from './utils/style'; @@ -18,7 +18,7 @@ interface OnResizeEvent { interface Props { layout: LayoutItem; - layoutLabel: (itemKey: string, rowIndex: number, colIndex: number)=> string; + layoutKeyToLabel: (key: string)=> string; onResize(event: OnResizeEvent): void; width?: number; height?: number; @@ -96,19 +96,14 @@ function ResizableLayout(props: Props) { props.onMoveButtonClick(event); }, [props.onMoveButtonClick]); - type ItemPosition = { - rowIndex: number; - colIndex: number; - }; - - const renderMoveControls = (item: LayoutItem, parent: LayoutItem | null, size: Size, { rowIndex, colIndex }: ItemPosition) => { + const renderMoveControls = (item: LayoutItem, parent: LayoutItem | null, size: Size) => { return ( { - return renderLayoutItem(props.layout, null, sizes, itemVisible(props.layout, props.moveMode), true, moveControlsOnly, { rowIndex: 0, colIndex: 0 }); + return renderLayoutItem(props.layout, null, sizes, itemVisible(props.layout, props.moveMode), true, moveControlsOnly); }; function renderMoveModeBox() { From c215796e2cdc7317bcf12f9f0f254b35a06db7ef Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Fri, 24 Jan 2025 09:55:45 -0800 Subject: [PATCH 12/19] Refactoring: Stronger types --- .../gui/ResizableLayout/ResizableLayout.tsx | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx index 4a26b79c6c2..2d6cbc66601 100644 --- a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx +++ b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx @@ -10,12 +10,19 @@ import MoveButtons, { MoveButtonClickEvent } from './MoveButtons'; import { StyledWrapperRoot, StyledMoveOverlay, MoveModeRootMessage } from './utils/style'; import { Resizable, ResizeCallback, ResizeStartCallback } from 're-resizable'; import Dialog from '../Dialog'; -const EventEmitter = require('events'); +import * as EventEmitter from 'events'; interface OnResizeEvent { layout: LayoutItem; } +interface ResizedItem { + key: string; + initialWidth: number; + initialHeight: number; + maxSize: Size; +} + interface Props { layout: LayoutItem; layoutKeyToLabel: (key: string)=> string; @@ -36,8 +43,7 @@ function itemVisible(item: LayoutItem, moveMode: boolean) { } function renderContainer(item: LayoutItem, parent: LayoutItem | null, sizes: LayoutItemSizes, resizedItemMaxSize: Size | null, onResizeStart: ResizeStartCallback, onResize: ResizeCallback, onResizeStop: ResizeCallback, children: React.ReactNode[], isLastChild: boolean, moveMode: boolean) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - const style: any = { + const style: React.CSSProperties = { display: itemVisible(item, moveMode) ? 'flex' : 'none', flexDirection: item.direction, }; @@ -87,8 +93,7 @@ function renderContainer(item: LayoutItem, parent: LayoutItem | null, sizes: Lay function ResizableLayout(props: Props) { const eventEmitter = useRef(new EventEmitter()); - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - const [resizedItem, setResizedItem] = useState(null); + const [resizedItem, setResizedItem] = useState(null); const lastUsedMoveButtonKey = useRef(null); const onMoveButtonClick = useCallback((event: MoveButtonClickEvent) => { @@ -126,17 +131,16 @@ function ResizableLayout(props: Props) { function renderLayoutItem( item: LayoutItem, parent: LayoutItem | null, sizes: LayoutItemSizes, isVisible: boolean, isLastChild: boolean, onlyMoveControls: boolean, ): React.ReactNode { - function onResizeStart() { + const onResizeStart: ResizeStartCallback = () => { setResizedItem({ key: item.key, initialWidth: sizes[item.key].width, initialHeight: sizes[item.key].height, maxSize: calculateMaxSizeAvailableForItem(item, parent, sizes), }); - } + }; - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - function onResize(_event: any, direction: string, _refToElement: any, delta: any) { + const onResize: ResizeCallback = (_event, direction, _refToElement, delta) => { const newWidth = Math.max(itemMinWidth, resizedItem.initialWidth + delta.width); const newHeight = Math.max(itemMinHeight, resizedItem.initialHeight + delta.height); @@ -156,13 +160,12 @@ function ResizableLayout(props: Props) { props.onResize({ layout: newLayout }); eventEmitter.current.emit('resize'); - } + }; - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - function onResizeStop(_event: any, _direction: any, _refToElement: any, delta: any) { + const onResizeStop: ResizeCallback = (_event, _direction, _refToElement, delta) => { onResize(_event, _direction, _refToElement, delta); setResizedItem(null); - } + }; const resizedItemMaxSize = resizedItem && item.key === resizedItem.key ? resizedItem.maxSize : null; if (!item.children) { From a5e64a4496f136eaa41e55fb513a2fb990cfbe08 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Fri, 24 Jan 2025 10:08:55 -0800 Subject: [PATCH 13/19] Refactoring: Convert function -> component --- .eslintignore | 1 + .gitignore | 1 + .../ResizableLayout/LayoutItemContainer.tsx | 69 +++++++++++++++++++ .../gui/ResizableLayout/ResizableLayout.tsx | 63 ++++------------- 4 files changed, 83 insertions(+), 51 deletions(-) create mode 100644 packages/app-desktop/gui/ResizableLayout/LayoutItemContainer.tsx diff --git a/.eslintignore b/.eslintignore index 569cda587dc..429277dfeef 100644 --- a/.eslintignore +++ b/.eslintignore @@ -355,6 +355,7 @@ packages/app-desktop/gui/PasswordInput/types.js packages/app-desktop/gui/PdfViewer.js packages/app-desktop/gui/PluginNotification/PluginNotification.js packages/app-desktop/gui/PromptDialog.js +packages/app-desktop/gui/ResizableLayout/LayoutItemContainer.js packages/app-desktop/gui/ResizableLayout/MoveButtons.js packages/app-desktop/gui/ResizableLayout/ResizableLayout.js packages/app-desktop/gui/ResizableLayout/utils/findItemByKey.js diff --git a/.gitignore b/.gitignore index 8145c35252a..b5e2921ee29 100644 --- a/.gitignore +++ b/.gitignore @@ -330,6 +330,7 @@ packages/app-desktop/gui/PasswordInput/types.js packages/app-desktop/gui/PdfViewer.js packages/app-desktop/gui/PluginNotification/PluginNotification.js packages/app-desktop/gui/PromptDialog.js +packages/app-desktop/gui/ResizableLayout/LayoutItemContainer.js packages/app-desktop/gui/ResizableLayout/MoveButtons.js packages/app-desktop/gui/ResizableLayout/ResizableLayout.js packages/app-desktop/gui/ResizableLayout/utils/findItemByKey.js diff --git a/packages/app-desktop/gui/ResizableLayout/LayoutItemContainer.tsx b/packages/app-desktop/gui/ResizableLayout/LayoutItemContainer.tsx new file mode 100644 index 00000000000..0012d387531 --- /dev/null +++ b/packages/app-desktop/gui/ResizableLayout/LayoutItemContainer.tsx @@ -0,0 +1,69 @@ +import * as React from 'react'; +import { Resizable, ResizeCallback, ResizeStartCallback, Size } from 're-resizable'; +import { LayoutItem } from './utils/types'; +import { itemMinHeight, itemMinWidth, itemSize, LayoutItemSizes } from './utils/useLayoutItemSizes'; + +interface Props { + item: LayoutItem; + parent: LayoutItem|null; + sizes: LayoutItemSizes; + resizedItemMaxSize: Size|null; + onResizeStart: ResizeStartCallback; + onResize: ResizeCallback; + onResizeStop: ResizeCallback; + children: React.ReactNode; + isLastChild: boolean; + visible: boolean; +} + +const LayoutItemContainer: React.FC = ({ + item, visible, parent, sizes, resizedItemMaxSize, onResize, onResizeStart, onResizeStop, children, isLastChild, +}) => { + const style: React.CSSProperties = { + display: visible ? 'flex' : 'none', + flexDirection: item.direction, + }; + + const size: Size = itemSize(item, parent, sizes, true); + + const className = `resizableLayoutItem rli-${item.key}`; + if (item.resizableRight || item.resizableBottom) { + const enable = { + top: false, + right: !!item.resizableRight && !isLastChild, + bottom: !!item.resizableBottom && !isLastChild, + left: false, + topRight: false, + bottomRight: false, + bottomLeft: false, + topLeft: false, + }; + + return ( + + {children} + + ); + } else { + return ( +
+ {children} +
+ ); + } +}; + +export default LayoutItemContainer; diff --git a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx index 2d6cbc66601..9c4bf6fce4a 100644 --- a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx +++ b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx @@ -8,9 +8,10 @@ import { Size, LayoutItem } from './utils/types'; import { canMove, MoveDirection } from './utils/movements'; import MoveButtons, { MoveButtonClickEvent } from './MoveButtons'; import { StyledWrapperRoot, StyledMoveOverlay, MoveModeRootMessage } from './utils/style'; -import { Resizable, ResizeCallback, ResizeStartCallback } from 're-resizable'; +import type { ResizeCallback, ResizeStartCallback } from 're-resizable'; import Dialog from '../Dialog'; import * as EventEmitter from 'events'; +import LayoutItemContainer from './LayoutItemContainer'; interface OnResizeEvent { layout: LayoutItem; @@ -42,54 +43,6 @@ function itemVisible(item: LayoutItem, moveMode: boolean) { return item.visible !== false; } -function renderContainer(item: LayoutItem, parent: LayoutItem | null, sizes: LayoutItemSizes, resizedItemMaxSize: Size | null, onResizeStart: ResizeStartCallback, onResize: ResizeCallback, onResizeStop: ResizeCallback, children: React.ReactNode[], isLastChild: boolean, moveMode: boolean) { - const style: React.CSSProperties = { - display: itemVisible(item, moveMode) ? 'flex' : 'none', - flexDirection: item.direction, - }; - - const size: Size = itemSize(item, parent, sizes, true); - - const className = `resizableLayoutItem rli-${item.key}`; - if (item.resizableRight || item.resizableBottom) { - const enable = { - top: false, - right: !!item.resizableRight && !isLastChild, - bottom: !!item.resizableBottom && !isLastChild, - left: false, - topRight: false, - bottomRight: false, - bottomLeft: false, - topLeft: false, - }; - - return ( - - {children} - - ); - } else { - return ( -
- {children} -
- ); - } -} - function ResizableLayout(props: Props) { const eventEmitter = useRef(new EventEmitter()); @@ -168,6 +121,10 @@ function ResizableLayout(props: Props) { }; const resizedItemMaxSize = resizedItem && item.key === resizedItem.key ? resizedItem.maxSize : null; + const visible = itemVisible(item, props.moveMode); + const itemContainerProps = { + item, parent, sizes, resizedItemMaxSize, onResizeStart, onResizeStop, onResize, isLastChild, visible, + }; if (!item.children) { const size = itemSize(item, parent, sizes, false); @@ -179,7 +136,9 @@ function ResizableLayout(props: Props) { }); const wrapper = onlyMoveControls ? renderMoveControls(item, parent, size) : renderItemWrapper(comp, item, size); - return renderContainer(item, parent, sizes, resizedItemMaxSize, onResizeStart, onResize, onResizeStop, [wrapper], isLastChild, props.moveMode); + return + {wrapper} + ; } else { const childrenComponents = []; for (let i = 0; i < item.children.length; i++) { @@ -189,7 +148,9 @@ function ResizableLayout(props: Props) { ); } - return renderContainer(item, parent, sizes, resizedItemMaxSize, onResizeStart, onResize, onResizeStop, childrenComponents, isLastChild, props.moveMode); + return + {childrenComponents} + ; } } From 4180890cec743dc2eb4cbace2501c95d0fd4acd7 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Fri, 24 Jan 2025 10:21:43 -0800 Subject: [PATCH 14/19] Refactoring, scan resize screen in integration tests --- .eslintignore | 1 + .gitignore | 1 + .../gui/ResizableLayout/ResizableLayout.tsx | 2 +- ...log.scss => change-app-layout-dialog.scss} | 2 +- packages/app-desktop/gui/styles/index.scss | 2 +- .../models/ChangeAppLayoutScreen.ts | 23 +++++++++++++++++++ .../integration-tests/models/MainScreen.ts | 3 +++ .../integration-tests/wcag.spec.ts | 6 +++++ 8 files changed, 37 insertions(+), 3 deletions(-) rename packages/app-desktop/gui/styles/{resizable-panels-move-dialog.scss => change-app-layout-dialog.scss} (85%) create mode 100644 packages/app-desktop/integration-tests/models/ChangeAppLayoutScreen.ts diff --git a/.eslintignore b/.eslintignore index 429277dfeef..7d687387d51 100644 --- a/.eslintignore +++ b/.eslintignore @@ -501,6 +501,7 @@ packages/app-desktop/gulpfile.js packages/app-desktop/integration-tests/goToAnything.spec.js packages/app-desktop/integration-tests/main.spec.js packages/app-desktop/integration-tests/markdownEditor.spec.js +packages/app-desktop/integration-tests/models/ChangeAppLayoutScreen.js packages/app-desktop/integration-tests/models/GoToAnything.js packages/app-desktop/integration-tests/models/MainScreen.js packages/app-desktop/integration-tests/models/NoteEditorScreen.js diff --git a/.gitignore b/.gitignore index b5e2921ee29..3170842db84 100644 --- a/.gitignore +++ b/.gitignore @@ -476,6 +476,7 @@ packages/app-desktop/gulpfile.js packages/app-desktop/integration-tests/goToAnything.spec.js packages/app-desktop/integration-tests/main.spec.js packages/app-desktop/integration-tests/markdownEditor.spec.js +packages/app-desktop/integration-tests/models/ChangeAppLayoutScreen.js packages/app-desktop/integration-tests/models/GoToAnything.js packages/app-desktop/integration-tests/models/MainScreen.js packages/app-desktop/integration-tests/models/NoteEditorScreen.js diff --git a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx index 9c4bf6fce4a..522b7c74ef1 100644 --- a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx +++ b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx @@ -167,7 +167,7 @@ function ResizableLayout(props: Props) { function renderMoveModeBox() { return
- + {props.moveModeMessage} {renderRoot(true)} diff --git a/packages/app-desktop/gui/styles/resizable-panels-move-dialog.scss b/packages/app-desktop/gui/styles/change-app-layout-dialog.scss similarity index 85% rename from packages/app-desktop/gui/styles/resizable-panels-move-dialog.scss rename to packages/app-desktop/gui/styles/change-app-layout-dialog.scss index 3685607e056..5b445c1838d 100644 --- a/packages/app-desktop/gui/styles/resizable-panels-move-dialog.scss +++ b/packages/app-desktop/gui/styles/change-app-layout-dialog.scss @@ -1,4 +1,4 @@ -.resizable-panels-move-dialog { +.change-app-layout-dialog { padding: 0; > .content { diff --git a/packages/app-desktop/gui/styles/index.scss b/packages/app-desktop/gui/styles/index.scss index 97b49e00381..e21bc051199 100644 --- a/packages/app-desktop/gui/styles/index.scss +++ b/packages/app-desktop/gui/styles/index.scss @@ -11,4 +11,4 @@ @use './dialog-anchor-node.scss'; @use './note-editor-wrapper.scss'; @use './text-input.scss'; -@use './resizable-panels-move-dialog.scss'; +@use './change-app-layout-dialog.scss'; diff --git a/packages/app-desktop/integration-tests/models/ChangeAppLayoutScreen.ts b/packages/app-desktop/integration-tests/models/ChangeAppLayoutScreen.ts new file mode 100644 index 00000000000..681bd9a123e --- /dev/null +++ b/packages/app-desktop/integration-tests/models/ChangeAppLayoutScreen.ts @@ -0,0 +1,23 @@ + +import { ElectronApplication, Locator, Page } from '@playwright/test'; +import MainScreen from './MainScreen'; +import activateMainMenuItem from '../util/activateMainMenuItem'; + +export default class ChangeAppLayoutScreen { + public readonly containerLocator: Locator; + + public constructor(page: Page, private readonly mainScreen: MainScreen) { + this.containerLocator = page.locator('.change-app-layout-dialog[open]'); + } + + public async open(electronApp: ElectronApplication) { + await this.mainScreen.waitFor(); + await activateMainMenuItem(electronApp, 'Change application layout'); + + return this.waitFor(); + } + + public async waitFor() { + await this.containerLocator.waitFor(); + } +} diff --git a/packages/app-desktop/integration-tests/models/MainScreen.ts b/packages/app-desktop/integration-tests/models/MainScreen.ts index 56f86a10e9a..f0fd733f390 100644 --- a/packages/app-desktop/integration-tests/models/MainScreen.ts +++ b/packages/app-desktop/integration-tests/models/MainScreen.ts @@ -6,6 +6,7 @@ import GoToAnything from './GoToAnything'; import setFilePickerResponse from '../util/setFilePickerResponse'; import NoteList from './NoteList'; import { expect } from '../util/test'; +import ChangeAppLayoutScreen from './ChangeAppLayoutScreen'; export default class MainScreen { public readonly newNoteButton: Locator; @@ -14,6 +15,7 @@ export default class MainScreen { public readonly dialog: Locator; public readonly noteEditor: NoteEditorScreen; public readonly goToAnything: GoToAnything; + public readonly changeLayoutScreen: ChangeAppLayoutScreen; public constructor(private page: Page) { this.newNoteButton = page.locator('.new-note-button'); @@ -22,6 +24,7 @@ export default class MainScreen { this.dialog = page.locator('.dialog-modal-layer'); this.noteEditor = new NoteEditorScreen(page); this.goToAnything = new GoToAnything(page, this); + this.changeLayoutScreen = new ChangeAppLayoutScreen(page, this); } public async setup() { diff --git a/packages/app-desktop/integration-tests/wcag.spec.ts b/packages/app-desktop/integration-tests/wcag.spec.ts index 600c8c5aeb1..3f851dcf32e 100644 --- a/packages/app-desktop/integration-tests/wcag.spec.ts +++ b/packages/app-desktop/integration-tests/wcag.spec.ts @@ -67,5 +67,11 @@ test.describe('wcag', () => { await expectNoViolations(mainWindow); }); + + test('should not detect significant issues in the change app layout screen', async ({ mainWindow, electronApp }) => { + const mainScreen = await new MainScreen(mainWindow).setup(); + await mainScreen.changeLayoutScreen.open(electronApp); + await expectNoViolations(mainWindow); + }); }); From 677f4c360279d38e3ab11918f4cf209bfefe185e Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Fri, 24 Jan 2025 10:46:06 -0800 Subject: [PATCH 15/19] Fix unique key error --- packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx index 522b7c74ef1..6f964696981 100644 --- a/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx +++ b/packages/app-desktop/gui/ResizableLayout/ResizableLayout.tsx @@ -123,7 +123,7 @@ function ResizableLayout(props: Props) { const resizedItemMaxSize = resizedItem && item.key === resizedItem.key ? resizedItem.maxSize : null; const visible = itemVisible(item, props.moveMode); const itemContainerProps = { - item, parent, sizes, resizedItemMaxSize, onResizeStart, onResizeStop, onResize, isLastChild, visible, + key: item.key, item, parent, sizes, resizedItemMaxSize, onResizeStart, onResizeStop, onResize, isLastChild, visible, }; if (!item.children) { const size = itemSize(item, parent, sizes, false); From 2f0bc12b3bdbe5d60e07a97808ad36e704890ed1 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Fri, 24 Jan 2025 11:05:39 -0800 Subject: [PATCH 16/19] Use group role rather than manually adding aria-describedby, add test --- .eslintignore | 1 + .gitignore | 1 + .../gui/ResizableLayout/MoveButtons.tsx | 3 +-- .../integration-tests/resizableLayout.spec.ts | 22 +++++++++++++++++++ 4 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 packages/app-desktop/integration-tests/resizableLayout.spec.ts diff --git a/.eslintignore b/.eslintignore index 7d687387d51..6f4b65cfa3b 100644 --- a/.eslintignore +++ b/.eslintignore @@ -510,6 +510,7 @@ packages/app-desktop/integration-tests/models/SettingsScreen.js packages/app-desktop/integration-tests/models/Sidebar.js packages/app-desktop/integration-tests/noteList.spec.js packages/app-desktop/integration-tests/pluginApi.spec.js +packages/app-desktop/integration-tests/resizableLayout.spec.js packages/app-desktop/integration-tests/richTextEditor.spec.js packages/app-desktop/integration-tests/settings.spec.js packages/app-desktop/integration-tests/sidebar.spec.js diff --git a/.gitignore b/.gitignore index 3170842db84..5aeb7099fe7 100644 --- a/.gitignore +++ b/.gitignore @@ -485,6 +485,7 @@ packages/app-desktop/integration-tests/models/SettingsScreen.js packages/app-desktop/integration-tests/models/Sidebar.js packages/app-desktop/integration-tests/noteList.spec.js packages/app-desktop/integration-tests/pluginApi.spec.js +packages/app-desktop/integration-tests/resizableLayout.spec.js packages/app-desktop/integration-tests/richTextEditor.spec.js packages/app-desktop/integration-tests/settings.spec.js packages/app-desktop/integration-tests/sidebar.spec.js diff --git a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx index 6ef203b51ed..896792e184c 100644 --- a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx +++ b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx @@ -106,14 +106,13 @@ export default function MoveButtons(props: Props) { level={ButtonLevel.Primary} iconName={`fas fa-arrow-${dir}`} iconLabel={iconLabel(dir)} - aria-describedby={descriptionId} autoFocus={autoFocusDirection === dir} onClick={() => onButtonClick(dir)} />; } return ( - + {renderButton(MoveDirection.Up)} diff --git a/packages/app-desktop/integration-tests/resizableLayout.spec.ts b/packages/app-desktop/integration-tests/resizableLayout.spec.ts new file mode 100644 index 00000000000..d448bcad230 --- /dev/null +++ b/packages/app-desktop/integration-tests/resizableLayout.spec.ts @@ -0,0 +1,22 @@ + +import { test, expect } from './util/test'; +import MainScreen from './models/MainScreen'; + +test.describe('resizableLayout', () => { + test('right/left buttons should retain keyboard focus after use', async ({ electronApp, mainWindow }) => { + const mainScreen = await new MainScreen(mainWindow).setup(); + const changeLayoutScreen = mainScreen.changeLayoutScreen; + await changeLayoutScreen.open(electronApp); + + const moveSidebarControls = changeLayoutScreen.containerLocator.getByRole('group', { name: 'Sidebar' }); + const moveSidebarRight = moveSidebarControls.getByRole('button', { name: 'Move right' }); + + await expect(moveSidebarRight).not.toBeDisabled(); + + // Should refocus (or keep focused) after clicking + await moveSidebarRight.click(); + await expect(moveSidebarRight).toBeFocused(); + await moveSidebarRight.click(); + await expect(moveSidebarRight).toBeFocused(); + }); +}); From 72e054c414b40e9af5f9c7202e84303359504e5f Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Fri, 24 Jan 2025 11:17:16 -0800 Subject: [PATCH 17/19] Fix reset layout dialog is shown behind the app layout change screen --- .../gui/WindowCommandsAndDialogs/commands/resetLayout.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/app-desktop/gui/WindowCommandsAndDialogs/commands/resetLayout.ts b/packages/app-desktop/gui/WindowCommandsAndDialogs/commands/resetLayout.ts index 6bb2f6002fa..950822793b9 100644 --- a/packages/app-desktop/gui/WindowCommandsAndDialogs/commands/resetLayout.ts +++ b/packages/app-desktop/gui/WindowCommandsAndDialogs/commands/resetLayout.ts @@ -1,6 +1,6 @@ import { CommandRuntime, CommandDeclaration, CommandContext } from '@joplin/lib/services/CommandService'; import { _ } from '@joplin/lib/locale'; -import dialogs from '../../dialogs'; +import shim from '@joplin/lib/shim'; export const declaration: CommandDeclaration = { name: 'resetLayout', @@ -12,7 +12,7 @@ export const runtime = (): CommandRuntime => { execute: async (context: CommandContext) => { const message = _('Are you sure you want to return to the default layout? The current layout configuration will be lost.'); - const isConfirmed = await dialogs.confirm(message); + const isConfirmed = await shim.showConfirmationDialog(message); if (!isConfirmed) return; From 87dd7674972014fba9353617571c1b31d2ceae71 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Fri, 24 Jan 2025 11:26:24 -0800 Subject: [PATCH 18/19] Code formatting --- packages/app-desktop/gui/Button/Button.tsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/app-desktop/gui/Button/Button.tsx b/packages/app-desktop/gui/Button/Button.tsx index 05bfa826659..26d844da1f5 100644 --- a/packages/app-desktop/gui/Button/Button.tsx +++ b/packages/app-desktop/gui/Button/Button.tsx @@ -18,11 +18,8 @@ export enum ButtonSize { Normal = 2, } -type BaseButtonProps = Omit< -React.DetailedHTMLProps, HTMLButtonElement>, -'onClick' ->; -interface Props extends BaseButtonProps { +type ReactButtonProps = React.DetailedHTMLProps, HTMLButtonElement>; +interface Props extends Omit { title?: string; iconName?: string; level?: ButtonLevel; From 60037901f9d480f8e905840b47db1973ab0010c4 Mon Sep 17 00:00:00 2001 From: Henry Heino Date: Fri, 24 Jan 2025 11:34:46 -0800 Subject: [PATCH 19/19] Improve comment --- packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx index 896792e184c..515040f8416 100644 --- a/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx +++ b/packages/app-desktop/gui/ResizableLayout/MoveButtons.tsx @@ -49,8 +49,9 @@ interface Props { canMoveUp: boolean; canMoveDown: boolean; - // A buttonKey to auto-focus. As one of the move buttons can change the app's layout, - // it's important to refocus the last-clicked button. + // Specifies which button to auto-focus (if any). Clicking a "Move ..." button changes the app's layout. By default, this + // causes focus to jump to the start of the move dialog. Providing the key of the last-clicked button allows focus + // to be restored after changing the app layout: autoFocusKey: ButtonKey|null; }