From 40dccfcdd1b7e0b7aea051d8bb0c05d97a388378 Mon Sep 17 00:00:00 2001 From: dwelle Date: Sat, 5 Aug 2023 12:59:47 +0200 Subject: [PATCH] narrow down & consolidate group selecting helper --- src/actions/actionDuplicateSelection.tsx | 3 +- src/actions/actionGroup.tsx | 2 +- src/actions/actionSelectAll.ts | 13 +- src/components/App.tsx | 248 ++++++++++++----------- src/groups.ts | 116 +++++------ 5 files changed, 194 insertions(+), 188 deletions(-) diff --git a/src/actions/actionDuplicateSelection.tsx b/src/actions/actionDuplicateSelection.tsx index dda172f33..a21260d5b 100644 --- a/src/actions/actionDuplicateSelection.tsx +++ b/src/actions/actionDuplicateSelection.tsx @@ -263,8 +263,7 @@ const duplicateElements = ( ...appState, ...selectGroupsForSelectedElements( { - ...appState, - selectedGroupIds: {}, + editingGroupId: appState.editingGroupId, selectedElementIds: nextElementsToSelect.reduce( (acc: Record, element) => { if (!isBoundToContainer(element)) { diff --git a/src/actions/actionGroup.tsx b/src/actions/actionGroup.tsx index c4a8f603f..8ee84ac7b 100644 --- a/src/actions/actionGroup.tsx +++ b/src/actions/actionGroup.tsx @@ -215,7 +215,7 @@ export const actionUngroup = register({ }); const updateAppState = selectGroupsForSelectedElements( - { ...appState, selectedGroupIds: {} }, + appState, getNonDeletedElements(nextElements), appState, null, diff --git a/src/actions/actionSelectAll.ts b/src/actions/actionSelectAll.ts index 6b30991f7..49f5072ce 100644 --- a/src/actions/actionSelectAll.ts +++ b/src/actions/actionSelectAll.ts @@ -32,13 +32,6 @@ export const actionSelectAll = register({ ...appState, ...selectGroupsForSelectedElements( { - ...appState, - selectedLinearElement: - // single linear element selected - Object.keys(selectedElementIds).length === 1 && - isLinearElement(elements[0]) - ? new LinearElementEditor(elements[0], app.scene) - : null, editingGroupId: null, selectedElementIds, }, @@ -46,6 +39,12 @@ export const actionSelectAll = register({ appState, app, ), + selectedLinearElement: + // single linear element selected + Object.keys(selectedElementIds).length === 1 && + isLinearElement(elements[0]) + ? new LinearElementEditor(elements[0], app.scene) + : null, }, commitToHistory: true, }; diff --git a/src/components/App.tsx b/src/components/App.tsx index 4f767296b..a9591428a 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -177,7 +177,6 @@ import { getSelectedGroupIds, isElementInGroup, isSelectedViaGroup, - selectGroups, selectGroupsForSelectedElements, } from "../groups"; import History from "../history"; @@ -1706,7 +1705,7 @@ class App extends React.Component { this.library.destroy(); clearTimeout(touchTimeout); isSomeElementSelected.clearCache(); - selectGroups.clearCache(); + selectGroupsForSelectedElements.clearCache(); touchTimeout = 0; } @@ -2300,35 +2299,37 @@ class App extends React.Component { excludeElementsInFramesFromSelection(newElements); this.setState( - selectGroupsForSelectedElements( - { - ...this.state, - // keep sidebar (presumably the library) open if it's docked and - // can fit. - // - // Note, we should close the sidebar only if we're dropping items - // from library, not when pasting from clipboard. Alas. - openSidebar: - this.state.openSidebar && - this.device.canDeviceFitSidebar && - jotaiStore.get(isSidebarDockedAtom) - ? this.state.openSidebar - : null, - selectedElementIds: nextElementsToSelect.reduce( - (acc: Record, element) => { - if (!isBoundToContainer(element)) { - acc[element.id] = true; - } - return acc; - }, - {}, - ), - selectedGroupIds: {}, - }, - this.scene.getNonDeletedElements(), - this.state, - this, - ), + { + ...this.state, + // keep sidebar (presumably the library) open if it's docked and + // can fit. + // + // Note, we should close the sidebar only if we're dropping items + // from library, not when pasting from clipboard. Alas. + openSidebar: + this.state.openSidebar && + this.device.canDeviceFitSidebar && + jotaiStore.get(isSidebarDockedAtom) + ? this.state.openSidebar + : null, + ...selectGroupsForSelectedElements( + { + editingGroupId: null, + selectedElementIds: nextElementsToSelect.reduce( + (acc: Record, element) => { + if (!isBoundToContainer(element)) { + acc[element.id] = true; + } + return acc; + }, + {}, + ), + }, + this.scene.getNonDeletedElements(), + this.state, + this, + ), + }, () => { if (opts.files) { this.addNewImagesToImageCache(); @@ -3589,19 +3590,18 @@ class App extends React.Component { getSelectedGroupIdForElement(hitElement, this.state.selectedGroupIds); if (selectedGroupId) { - this.setState((prevState) => - selectGroupsForSelectedElements( + this.setState((prevState) => ({ + ...prevState, + ...selectGroupsForSelectedElements( { - ...prevState, editingGroupId: selectedGroupId, selectedElementIds: { [hitElement!.id]: true }, - selectedGroupIds: {}, }, this.scene.getNonDeletedElements(), prevState, this, ), - ); + })); return; } } @@ -5096,19 +5096,21 @@ class App extends React.Component { } } - return selectGroupsForSelectedElements( - { - ...prevState, - selectedElementIds: nextSelectedElementIds, - showHyperlinkPopup: - hitElement.link || isEmbeddableElement(hitElement) - ? "info" - : false, - }, - this.scene.getNonDeletedElements(), - prevState, - this, - ); + return { + ...selectGroupsForSelectedElements( + { + editingGroupId: prevState.editingGroupId, + selectedElementIds: nextSelectedElementIds, + }, + this.scene.getNonDeletedElements(), + prevState, + this, + ), + showHyperlinkPopup: + hitElement.link || isEmbeddableElement(hitElement) + ? "info" + : false, + }; }); pointerDownState.hit.wasAddedToSelection = true; } @@ -6028,34 +6030,36 @@ class App extends React.Component { } } - return selectGroupsForSelectedElements( - { - ...prevState, - ...(!shouldReuseSelection && { - selectedGroupIds: {}, - editingGroupId: null, - }), - selectedElementIds: nextSelectedElementIds, - showHyperlinkPopup: - elementsWithinSelection.length === 1 && - (elementsWithinSelection[0].link || - isEmbeddableElement(elementsWithinSelection[0])) - ? "info" - : false, - // select linear element only when we haven't box-selected anything else - selectedLinearElement: - elementsWithinSelection.length === 1 && - isLinearElement(elementsWithinSelection[0]) - ? new LinearElementEditor( - elementsWithinSelection[0], - this.scene, - ) - : null, - }, - this.scene.getNonDeletedElements(), - prevState, - this, - ); + prevState = !shouldReuseSelection + ? { ...prevState, selectedGroupIds: {}, editingGroupId: null } + : prevState; + + return { + ...selectGroupsForSelectedElements( + { + editingGroupId: prevState.editingGroupId, + selectedElementIds: nextSelectedElementIds, + }, + this.scene.getNonDeletedElements(), + prevState, + this, + ), + // select linear element only when we haven't box-selected anything else + selectedLinearElement: + elementsWithinSelection.length === 1 && + isLinearElement(elementsWithinSelection[0]) + ? new LinearElementEditor( + elementsWithinSelection[0], + this.scene, + ) + : null, + showHyperlinkPopup: + elementsWithinSelection.length === 1 && + (elementsWithinSelection[0].link || + isEmbeddableElement(elementsWithinSelection[0])) + ? "info" + : false, + }; }); } } @@ -6656,24 +6660,26 @@ class App extends React.Component { { selectedElementIds: newSelectedElementIds }, ); - return selectGroupsForSelectedElements( - { - ...prevState, - selectedElementIds: newSelectedElementIds, - // set selectedLinearElement only if thats the only element selected - selectedLinearElement: - newSelectedElements.length === 1 && - isLinearElement(newSelectedElements[0]) - ? new LinearElementEditor( - newSelectedElements[0], - this.scene, - ) - : prevState.selectedLinearElement, - }, - this.scene.getNonDeletedElements(), - prevState, - this, - ); + return { + ...selectGroupsForSelectedElements( + { + editingGroupId: prevState.editingGroupId, + selectedElementIds: newSelectedElementIds, + }, + this.scene.getNonDeletedElements(), + prevState, + this, + ), + // set selectedLinearElement only if thats the only element selected + selectedLinearElement: + newSelectedElements.length === 1 && + isLinearElement(newSelectedElements[0]) + ? new LinearElementEditor( + newSelectedElements[0], + this.scene, + ) + : prevState.selectedLinearElement, + }; }); } } else if ( @@ -6701,19 +6707,21 @@ class App extends React.Component { delete nextSelectedElementIds[element.id]; }); - return selectGroupsForSelectedElements( - { - ...prevState, - selectedElementIds: nextSelectedElementIds, - showHyperlinkPopup: - hitElement.link || isEmbeddableElement(hitElement) - ? "info" - : false, - }, - this.scene.getNonDeletedElements(), - prevState, - this, - ); + return { + ...selectGroupsForSelectedElements( + { + editingGroupId: prevState.editingGroupId, + selectedElementIds: nextSelectedElementIds, + }, + this.scene.getNonDeletedElements(), + prevState, + this, + ), + showHyperlinkPopup: + hitElement.link || isEmbeddableElement(hitElement) + ? "info" + : false, + }; }); } else { // add element to selection while keeping prev elements selected @@ -6731,20 +6739,20 @@ class App extends React.Component { this.setState((prevState) => ({ ...selectGroupsForSelectedElements( { - ...prevState, + editingGroupId: prevState.editingGroupId, selectedElementIds: { [hitElement.id]: true }, - selectedLinearElement: - isLinearElement(hitElement) && - // Don't set `selectedLinearElement` if its same as the hitElement, this is mainly to prevent resetting the `hoverPointIndex` to -1. - // Future we should update the API to take care of setting the correct `hoverPointIndex` when initialized - prevState.selectedLinearElement?.elementId !== hitElement.id - ? new LinearElementEditor(hitElement, this.scene) - : prevState.selectedLinearElement, }, this.scene.getNonDeletedElements(), prevState, this, ), + selectedLinearElement: + isLinearElement(hitElement) && + // Don't set `selectedLinearElement` if its same as the hitElement, this is mainly to prevent resetting the `hoverPointIndex` to -1. + // Future we should update the API to take care of setting the correct `hoverPointIndex` when initialized + prevState.selectedLinearElement?.elementId !== hitElement.id + ? new LinearElementEditor(hitElement, this.scene) + : prevState.selectedLinearElement, })); } } @@ -7595,16 +7603,16 @@ class App extends React.Component { ...this.state, ...selectGroupsForSelectedElements( { - ...this.state, + editingGroupId: this.state.editingGroupId, selectedElementIds: { [element.id]: true }, - selectedLinearElement: isLinearElement(element) - ? new LinearElementEditor(element, this.scene) - : null, }, this.scene.getNonDeletedElements(), this.state, this, ), + selectedLinearElement: isLinearElement(element) + ? new LinearElementEditor(element, this.scene) + : null, } : this.state), showHyperlinkPopup: false, diff --git a/src/groups.ts b/src/groups.ts index 269566173..e7b222b3c 100644 --- a/src/groups.ts +++ b/src/groups.ts @@ -55,25 +55,29 @@ export const selectGroup = ( }; }; -export const selectGroups = (function () { +export const selectGroupsForSelectedElements = (function () { + type SelectGroupsReturnType = Pick< + InteractiveCanvasAppState, + "selectedGroupIds" | "editingGroupId" | "selectedElementIds" + >; + let lastSelectedElements: readonly NonDeleted[] | null = null; let lastElements: readonly NonDeleted[] | null = null; - let lastAppState: InteractiveCanvasAppState | null = null; + let lastReturnValue: SelectGroupsReturnType | null = null; - const ret = ( + const _selectGroups = ( selectedElements: readonly NonDeleted[], elements: readonly NonDeleted[], - appState: InteractiveCanvasAppState, - ): InteractiveCanvasAppState => { + appState: Pick, + ): SelectGroupsReturnType => { if ( - lastAppState !== undefined && + lastReturnValue !== undefined && elements === lastElements && selectedElements === lastSelectedElements && - appState.editingGroupId === lastAppState?.editingGroupId && - appState.selectedGroupIds === lastAppState?.selectedGroupIds + appState.editingGroupId === lastReturnValue?.editingGroupId ) { - return lastAppState; + return lastReturnValue; } const selectedGroupIds: Record = {}; @@ -126,8 +130,8 @@ export const selectGroups = (function () { lastElements = elements; lastSelectedElements = selectedElements; - lastAppState = { - ...appState, + lastReturnValue = { + editingGroupId: appState.editingGroupId, selectedGroupIds, selectedElementIds: { ...appState.selectedElementIds, @@ -135,16 +139,55 @@ export const selectGroups = (function () { }, }; - return lastAppState; + return lastReturnValue; }; - ret.clearCache = () => { + /** + * When you select an element, you often want to actually select the whole group it's in, unless + * you're currently editing that group. + */ + const selectGroupsForSelectedElements = ( + appState: Pick, + elements: readonly NonDeletedExcalidrawElement[], + prevAppState: InteractiveCanvasAppState, + /** + * supply null in cases where you don't have access to App instance and + * you don't care about optimizing selectElements retrieval + */ + app: AppClassProperties | null, + ): Pick< + InteractiveCanvasAppState, + "selectedGroupIds" | "editingGroupId" | "selectedElementIds" + > => { + const selectedElements = app + ? app.scene.getSelectedElements({ + selectedElementIds: appState.selectedElementIds, + // supplying elements explicitly in case we're passed non-state elements + elements, + }) + : getSelectedElements(elements, appState); + + if (!selectedElements.length) { + return { + selectedGroupIds: {}, + editingGroupId: null, + selectedElementIds: makeNextSelectedElementIds( + appState.selectedElementIds, + prevAppState, + ), + }; + } + + return _selectGroups(selectedElements, elements, appState); + }; + + selectGroupsForSelectedElements.clearCache = () => { lastElements = null; lastSelectedElements = null; - lastAppState = null; + lastReturnValue = null; }; - return ret; + return selectGroupsForSelectedElements; })(); /** @@ -171,49 +214,6 @@ export const getSelectedGroupIds = ( .filter(([groupId, isSelected]) => isSelected) .map(([groupId, isSelected]) => groupId); -/** - * When you select an element, you often want to actually select the whole group it's in, unless - * you're currently editing that group. - */ -export const selectGroupsForSelectedElements = ( - appState: InteractiveCanvasAppState, - elements: readonly NonDeletedExcalidrawElement[], - prevAppState: InteractiveCanvasAppState, - /** - * supply null in cases where you don't have access to App instance and - * you don't care about optimizing selectElements retrieval - */ - app: AppClassProperties | null, -): InteractiveCanvasAppState => { - let nextAppState: InteractiveCanvasAppState = { - ...appState, - selectedGroupIds: {}, - }; - - const selectedElements = app - ? app.scene.getSelectedElements({ - selectedElementIds: appState.selectedElementIds, - // supplying elements explicitly in case we're passed non-state elements - elements, - }) - : getSelectedElements(elements, appState); - - if (!selectedElements.length) { - return { - ...nextAppState, - editingGroupId: null, - selectedElementIds: makeNextSelectedElementIds( - nextAppState.selectedElementIds, - prevAppState, - ), - }; - } - - nextAppState = selectGroups(selectedElements, elements, appState); - - return nextAppState; -}; - // given a list of elements, return the the actual group ids that should be selected // or used to update the elements export const selectGroupsFromGivenElements = (