From 2cd7573c66ba0d230d4da875b8b1f8d1d54aea32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mitja=20Bezen=C5=A1ek?= Date: Wed, 24 Jan 2024 14:38:58 +0100 Subject: [PATCH] Only actions on selected shapes if we are in select tool. (#2617) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't want to allow actions that operate on selections when we are not in select tool. For example, you could use duplicate shape keyboard shortcut when in Hand tool and it would just create a shape out of nowhere (we don't clear selection when changing tools so it would duplicate the last selection). ### Change Type - [x] `patch` — Bug fix - [ ] `minor` — New feature - [ ] `major` — Breaking change - [ ] `dependencies` — Changes to package dependencies[^1] - [ ] `documentation` — Changes to the documentation only[^2] - [ ] `tests` — Changes to any test code only[^2] - [ ] `internal` — Any other changes that don't affect the published package[^2] - [ ] I don't know [^1]: publishes a `patch` release, for devDependencies use `internal` [^2]: will not publish a new version ### Test Plan 1. Select some shapes. 2. Switch to a different tool (like hand) 3. Try to duplicate, align,... via keyboard shortcuts. This should no longer work. - [ ] Unit Tests - [ ] End to end tests ### Release Notes - Disable actions that work on selections when we are not in select tool as it makes it not obvious what the target for these actions. --- .../tldraw/src/lib/ui/hooks/useActions.tsx | 74 +++++++++---------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/packages/tldraw/src/lib/ui/hooks/useActions.tsx b/packages/tldraw/src/lib/ui/hooks/useActions.tsx index b0ad9872f..74094a1a6 100644 --- a/packages/tldraw/src/lib/ui/hooks/useActions.tsx +++ b/packages/tldraw/src/lib/ui/hooks/useActions.tsx @@ -99,8 +99,8 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { return false } - function hasSelectedShapes() { - return editor.getSelectedShapeIds().length > 0 + function canApplySelectionAction() { + return editor.isIn('select') && editor.getSelectedShapeIds().length > 0 } const actionItems: TLUiActionItem[] = [ @@ -110,7 +110,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'link', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('edit-link', { source }) @@ -232,7 +232,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { label: 'action.toggle-auto-size', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('toggle-auto-size', { source }) @@ -300,7 +300,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { label: 'action.convert-to-bookmark', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return editor.batch(() => { @@ -344,7 +344,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { label: 'action.convert-to-embed', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('convert-to-embed', { source }) @@ -402,7 +402,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'duplicate', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('duplicate-shapes', { source }) @@ -428,7 +428,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'ungroup', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('ungroup-shapes', { source }) @@ -443,7 +443,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'group', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('group-shapes', { source }) @@ -463,7 +463,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '$!f', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return trackEvent('remove-frame', { source }) const selectedShapes = editor.getSelectedShapes() @@ -484,7 +484,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { label: 'action.fit-frame-to-content', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return trackEvent('fit-frame-to-content', { source }) const onlySelectedShape = editor.getOnlySelectedShape() @@ -501,7 +501,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'align-left', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('align-shapes', { operation: 'left', source }) @@ -517,7 +517,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'align-center-horizontal', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('align-shapes', { operation: 'center-horizontal', source }) @@ -532,7 +532,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'align-right', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('align-shapes', { operation: 'right', source }) @@ -548,7 +548,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'align-center-vertical', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('align-shapes', { operation: 'center-vertical', source }) @@ -563,7 +563,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '?W', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('align-shapes', { operation: 'top', source }) @@ -578,7 +578,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '?S', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('align-shapes', { operation: 'bottom', source }) @@ -594,7 +594,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '?!h', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('distribute-shapes', { operation: 'horizontal', source }) @@ -610,7 +610,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '?!V', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('distribute-shapes', { operation: 'vertical', source }) @@ -625,7 +625,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'stretch-horizontal', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('stretch-shapes', { operation: 'horizontal', source }) @@ -640,7 +640,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'stretch-vertical', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('stretch-shapes', { operation: 'vertical', source }) @@ -655,7 +655,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '!h', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('flip-shapes', { operation: 'horizontal', source }) @@ -670,7 +670,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '!v', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('flip-shapes', { operation: 'vertical', source }) @@ -684,7 +684,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'pack', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('pack-shapes', { source }) @@ -699,7 +699,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'stack-vertical', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('stack-shapes', { operation: 'vertical', source }) @@ -714,7 +714,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'stack-horizontal', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('stack-shapes', { operation: 'horizontal', source }) @@ -729,7 +729,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'bring-to-front', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('reorder-shapes', { operation: 'toFront', source }) @@ -744,7 +744,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '?]', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('reorder-shapes', { operation: 'forward', source }) @@ -759,7 +759,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '?[', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('reorder-shapes', { operation: 'backward', source }) @@ -774,7 +774,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '[', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('reorder-shapes', { operation: 'toBack', source }) @@ -788,7 +788,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '$x', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return editor.mark('cut') @@ -801,7 +801,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '$c', readonlyOk: true, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return copy(source) @@ -843,7 +843,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { label: 'action.select-none', readonlyOk: true, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('select-none-shapes', { source }) @@ -858,7 +858,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'trash', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('delete-shapes', { source }) @@ -872,7 +872,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'rotate-cw', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('rotate-cw', { source }) @@ -891,7 +891,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { icon: 'rotate-ccw', readonlyOk: false, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('rotate-ccw', { source }) @@ -951,7 +951,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { kbd: '!2', readonlyOk: true, onSelect(source) { - if (!hasSelectedShapes()) return + if (!canApplySelectionAction()) return if (mustGoBackToSelectToolFirst()) return trackEvent('zoom-to-selection', { source })