Only actions on selected shapes if we are in select tool. (#2617)

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.
This commit is contained in:
Mitja Bezenšek 2024-01-24 14:38:58 +01:00 committed by GitHub
parent 014a95cf51
commit 2cd7573c66
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -99,8 +99,8 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
return false return false
} }
function hasSelectedShapes() { function canApplySelectionAction() {
return editor.getSelectedShapeIds().length > 0 return editor.isIn('select') && editor.getSelectedShapeIds().length > 0
} }
const actionItems: TLUiActionItem<TLUiTranslationKey, TLUiIconType>[] = [ const actionItems: TLUiActionItem<TLUiTranslationKey, TLUiIconType>[] = [
@ -110,7 +110,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'link', icon: 'link',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('edit-link', { source }) trackEvent('edit-link', { source })
@ -232,7 +232,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
label: 'action.toggle-auto-size', label: 'action.toggle-auto-size',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('toggle-auto-size', { source }) trackEvent('toggle-auto-size', { source })
@ -300,7 +300,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
label: 'action.convert-to-bookmark', label: 'action.convert-to-bookmark',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
editor.batch(() => { editor.batch(() => {
@ -344,7 +344,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
label: 'action.convert-to-embed', label: 'action.convert-to-embed',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('convert-to-embed', { source }) trackEvent('convert-to-embed', { source })
@ -402,7 +402,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'duplicate', icon: 'duplicate',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('duplicate-shapes', { source }) trackEvent('duplicate-shapes', { source })
@ -428,7 +428,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'ungroup', icon: 'ungroup',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('ungroup-shapes', { source }) trackEvent('ungroup-shapes', { source })
@ -443,7 +443,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'group', icon: 'group',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('group-shapes', { source }) trackEvent('group-shapes', { source })
@ -463,7 +463,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '$!f', kbd: '$!f',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
trackEvent('remove-frame', { source }) trackEvent('remove-frame', { source })
const selectedShapes = editor.getSelectedShapes() const selectedShapes = editor.getSelectedShapes()
@ -484,7 +484,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
label: 'action.fit-frame-to-content', label: 'action.fit-frame-to-content',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
trackEvent('fit-frame-to-content', { source }) trackEvent('fit-frame-to-content', { source })
const onlySelectedShape = editor.getOnlySelectedShape() const onlySelectedShape = editor.getOnlySelectedShape()
@ -501,7 +501,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'align-left', icon: 'align-left',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('align-shapes', { operation: 'left', source }) trackEvent('align-shapes', { operation: 'left', source })
@ -517,7 +517,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'align-center-horizontal', icon: 'align-center-horizontal',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('align-shapes', { operation: 'center-horizontal', source }) trackEvent('align-shapes', { operation: 'center-horizontal', source })
@ -532,7 +532,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'align-right', icon: 'align-right',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('align-shapes', { operation: 'right', source }) trackEvent('align-shapes', { operation: 'right', source })
@ -548,7 +548,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'align-center-vertical', icon: 'align-center-vertical',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('align-shapes', { operation: 'center-vertical', source }) trackEvent('align-shapes', { operation: 'center-vertical', source })
@ -563,7 +563,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '?W', kbd: '?W',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('align-shapes', { operation: 'top', source }) trackEvent('align-shapes', { operation: 'top', source })
@ -578,7 +578,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '?S', kbd: '?S',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('align-shapes', { operation: 'bottom', source }) trackEvent('align-shapes', { operation: 'bottom', source })
@ -594,7 +594,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '?!h', kbd: '?!h',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('distribute-shapes', { operation: 'horizontal', source }) trackEvent('distribute-shapes', { operation: 'horizontal', source })
@ -610,7 +610,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '?!V', kbd: '?!V',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('distribute-shapes', { operation: 'vertical', source }) trackEvent('distribute-shapes', { operation: 'vertical', source })
@ -625,7 +625,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'stretch-horizontal', icon: 'stretch-horizontal',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('stretch-shapes', { operation: 'horizontal', source }) trackEvent('stretch-shapes', { operation: 'horizontal', source })
@ -640,7 +640,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'stretch-vertical', icon: 'stretch-vertical',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('stretch-shapes', { operation: 'vertical', source }) trackEvent('stretch-shapes', { operation: 'vertical', source })
@ -655,7 +655,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '!h', kbd: '!h',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('flip-shapes', { operation: 'horizontal', source }) trackEvent('flip-shapes', { operation: 'horizontal', source })
@ -670,7 +670,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '!v', kbd: '!v',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('flip-shapes', { operation: 'vertical', source }) trackEvent('flip-shapes', { operation: 'vertical', source })
@ -684,7 +684,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'pack', icon: 'pack',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('pack-shapes', { source }) trackEvent('pack-shapes', { source })
@ -699,7 +699,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'stack-vertical', icon: 'stack-vertical',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('stack-shapes', { operation: 'vertical', source }) trackEvent('stack-shapes', { operation: 'vertical', source })
@ -714,7 +714,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'stack-horizontal', icon: 'stack-horizontal',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('stack-shapes', { operation: 'horizontal', source }) trackEvent('stack-shapes', { operation: 'horizontal', source })
@ -729,7 +729,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'bring-to-front', icon: 'bring-to-front',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('reorder-shapes', { operation: 'toFront', source }) trackEvent('reorder-shapes', { operation: 'toFront', source })
@ -744,7 +744,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '?]', kbd: '?]',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('reorder-shapes', { operation: 'forward', source }) trackEvent('reorder-shapes', { operation: 'forward', source })
@ -759,7 +759,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '?[', kbd: '?[',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('reorder-shapes', { operation: 'backward', source }) trackEvent('reorder-shapes', { operation: 'backward', source })
@ -774,7 +774,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '[', kbd: '[',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('reorder-shapes', { operation: 'toBack', source }) trackEvent('reorder-shapes', { operation: 'toBack', source })
@ -788,7 +788,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '$x', kbd: '$x',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
editor.mark('cut') editor.mark('cut')
@ -801,7 +801,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '$c', kbd: '$c',
readonlyOk: true, readonlyOk: true,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
copy(source) copy(source)
@ -843,7 +843,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
label: 'action.select-none', label: 'action.select-none',
readonlyOk: true, readonlyOk: true,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('select-none-shapes', { source }) trackEvent('select-none-shapes', { source })
@ -858,7 +858,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'trash', icon: 'trash',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('delete-shapes', { source }) trackEvent('delete-shapes', { source })
@ -872,7 +872,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'rotate-cw', icon: 'rotate-cw',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('rotate-cw', { source }) trackEvent('rotate-cw', { source })
@ -891,7 +891,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
icon: 'rotate-ccw', icon: 'rotate-ccw',
readonlyOk: false, readonlyOk: false,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('rotate-ccw', { source }) trackEvent('rotate-ccw', { source })
@ -951,7 +951,7 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) {
kbd: '!2', kbd: '!2',
readonlyOk: true, readonlyOk: true,
onSelect(source) { onSelect(source) {
if (!hasSelectedShapes()) return if (!canApplySelectionAction()) return
if (mustGoBackToSelectToolFirst()) return if (mustGoBackToSelectToolFirst()) return
trackEvent('zoom-to-selection', { source }) trackEvent('zoom-to-selection', { source })