From f185edcb76b5da78a8d40ce129861643e3d302d0 Mon Sep 17 00:00:00 2001 From: alex Date: Wed, 7 Feb 2024 15:57:10 +0000 Subject: [PATCH] Fix infinite cursor chat issue by partially reverting "reactive context menu overrides (#2697)" (#2775) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we made context menu overrides, we introduced two new issues: 1. the context menu on the main app now updates much more frequently than it should 2. every time it updates, it adds a new 'cursor chat' entry to the menu This reverts the part of that change that made the context menu reactive. This is the quick fix for us to hotfix, but i'm going to follow this up by restoring that functionality without those issues. ### Change Type - [x] `patch` — Bug fix [^1]: publishes a `patch` release, for devDependencies use `internal` [^2]: will not publish a new version --- .../src/lib/ui/hooks/useContextMenuSchema.tsx | 45 +++++++------------ .../tldraw/src/test/ui/ContextMenu.test.tsx | 22 ++++----- 2 files changed, 26 insertions(+), 41 deletions(-) diff --git a/packages/tldraw/src/lib/ui/hooks/useContextMenuSchema.tsx b/packages/tldraw/src/lib/ui/hooks/useContextMenuSchema.tsx index 87168ae79..5b6c91cef 100644 --- a/packages/tldraw/src/lib/ui/hooks/useContextMenuSchema.tsx +++ b/packages/tldraw/src/lib/ui/hooks/useContextMenuSchema.tsx @@ -87,8 +87,8 @@ export const TLUiContextMenuSchemaProvider = track(function TLUiContextMenuSchem editor.getSortedChildIdsForParent(onlySelectedShape).length > 0 const isShapeLocked = onlySelectedShape && editor.isShapeOrAncestorLocked(onlySelectedShape) - const contextTLUiMenuSchemaWithoutOverrides = useMemo(() => { - return compactMenuItems([ + const contextTLUiMenuSchema = useMemo(() => { + const contextTLUiMenuSchemaWithoutOverrides: TLUiMenuSchema = compactMenuItems([ menuGroup( 'selection', showAutoSizeToggle && menuItem(actions['toggle-auto-size']), @@ -204,6 +204,17 @@ export const TLUiContextMenuSchemaProvider = track(function TLUiContextMenuSchem ), oneSelected && !isShapeLocked && menuGroup('delete-group', menuItem(actions['delete'])), ]) + + if (!overrides) return contextTLUiMenuSchemaWithoutOverrides + return overrides(editor, contextTLUiMenuSchemaWithoutOverrides, { + actions, + oneSelected, + twoSelected, + threeSelected, + showAutoSizeToggle, + showUngroup: allowUngroup, + onlyFlippableShapeSelected, + }) }, [ actions, oneSelected, @@ -223,36 +234,10 @@ export const TLUiContextMenuSchemaProvider = track(function TLUiContextMenuSchem // oneEmbeddableBookmarkSelected, isTransparentBg, isShapeLocked, + editor, + overrides, ]) - const contextTLUiMenuSchema = useValue( - 'overrides', - () => { - if (!overrides) return contextTLUiMenuSchemaWithoutOverrides - return overrides(editor, contextTLUiMenuSchemaWithoutOverrides, { - actions, - oneSelected, - twoSelected, - threeSelected, - showAutoSizeToggle, - showUngroup: allowUngroup, - onlyFlippableShapeSelected, - }) - }, - [ - actions, - allowUngroup, - contextTLUiMenuSchemaWithoutOverrides, - editor, - oneSelected, - onlyFlippableShapeSelected, - overrides, - showAutoSizeToggle, - threeSelected, - twoSelected, - ] - ) - return ( {children} diff --git a/packages/tldraw/src/test/ui/ContextMenu.test.tsx b/packages/tldraw/src/test/ui/ContextMenu.test.tsx index caaa4086b..7da72ce84 100644 --- a/packages/tldraw/src/test/ui/ContextMenu.test.tsx +++ b/packages/tldraw/src/test/ui/ContextMenu.test.tsx @@ -1,5 +1,5 @@ import { fireEvent, screen } from '@testing-library/react' -import { createShapeId, noop } from '@tldraw/editor' +import { atom, createShapeId, noop } from '@tldraw/editor' import { act } from 'react-dom/test-utils' import { Tldraw } from '../../lib/Tldraw' import { TLUiOverrides } from '../../lib/ui/overrides' @@ -26,11 +26,11 @@ it('opens on right-click', async () => { expect(screen.queryByTestId('context-menu')).toBeNull() }) -it('updates overrides reactively', async () => { +it.failing('updates overrides reactively', async () => { + const count = atom('count', 1) const overrides: TLUiOverrides = { contextMenu: (editor, schema) => { - const items = editor.getSelectedShapeIds().length - if (items === 0) return schema + if (count.get() === 0) return schema return [ ...schema, { @@ -43,7 +43,7 @@ it('updates overrides reactively', async () => { id: 'tester', readonlyOk: true, onSelect: noop, - label: `Selected: ${items}`, + label: `Count: ${count.get()}`, }, }, ] @@ -61,14 +61,14 @@ it('updates overrides reactively', async () => { // check that the context menu item was added: await screen.findByTestId('menu-item.tester') - // It should disappear when we deselect all shapes: - await act(() => editor.setSelectedShapes([])) + // It should disappear when count is 0: + await act(() => count.set(0)) expect(screen.queryByTestId('menu-item.tester')).toBeNull() // It should update its label when it changes: - await act(() => editor.selectAll()) + await act(() => count.set(1)) const item = await screen.findByTestId('menu-item.tester') - expect(item.textContent).toBe('Selected: 1') - await act(() => editor.createShape({ id: createShapeId(), type: 'geo' }).selectAll()) - expect(item.textContent).toBe('Selected: 2') + expect(item.textContent).toBe('Count: 1') + await act(() => count.set(2)) + expect(item.textContent).toBe('Count: 2') })