diff --git a/e2e/playwright/basic-sketch.spec.ts b/e2e/playwright/basic-sketch.spec.ts index adf2270bf..d386bda7e 100644 --- a/e2e/playwright/basic-sketch.spec.ts +++ b/e2e/playwright/basic-sketch.spec.ts @@ -137,7 +137,7 @@ async function doBasicSketch( await page.waitForTimeout(100) } - await page.getByRole('button', { name: 'Length: open menu' }).click() + await page.getByRole('button', { name: 'constraints: open menu' }).click() await page.getByRole('button', { name: 'Equal Length' }).click() // Open the code pane. diff --git a/e2e/playwright/editor-tests.spec.ts b/e2e/playwright/editor-tests.spec.ts index de0c4bdcf..cfd34c12d 100644 --- a/e2e/playwright/editor-tests.spec.ts +++ b/e2e/playwright/editor-tests.spec.ts @@ -1353,4 +1353,51 @@ sketch001 = startSketchOn(XZ) 15 ) }) + + test(`test-toolbar-buttons`, async ({ + page, + homePage, + toolbar, + scene, + cmdBar, + }) => { + await test.step('Load an empty file', async () => { + await page.addInitScript(async () => { + localStorage.setItem('persistCode', '') + }) + await page.setBodyDimensions({ width: 1200, height: 500 }) + await homePage.goToModelingScene() + + // wait until scene is ready to be interacted with + await scene.connectionEstablished() + await scene.settled(cmdBar) + }) + + await test.step('Test toolbar button correct selection', async () => { + await toolbar.expectToolbarMode.toBe('modeling') + + await toolbar.startSketchPlaneSelection() + + // Click on a default plane + await page.mouse.click(700, 200) + + // tools cannot be selected immediately, couldn't find an event to await instead. + await page.waitForTimeout(1000) + + await toolbar.selectCenterRectangle() + + await expect(page.getByTestId('center-rectangle')).toHaveAttribute( + 'aria-pressed', + 'true' + ) + }) + + await test.step('Test Toolbar dropdown remembering last selection', async () => { + // Select another tool + await page.getByTestId('circle-center').click() + + // center-rectangle should still be the active option in the rectangle dropdown + await expect(page.getByTestId('center-rectangle')).toBeVisible() + }) + }) }) diff --git a/e2e/playwright/fixtures/toolbarFixture.ts b/e2e/playwright/fixtures/toolbarFixture.ts index 282ad87eb..8b98af630 100644 --- a/e2e/playwright/fixtures/toolbarFixture.ts +++ b/e2e/playwright/fixtures/toolbarFixture.ts @@ -169,7 +169,7 @@ export class ToolbarFixture { } selectCenterRectangle = async () => { await this.page - .getByRole('button', { name: 'caret down Corner rectangle:' }) + .getByRole('button', { name: 'caret down rectangles:' }) .click() await expect( this.page.getByTestId('dropdown-center-rectangle') @@ -178,7 +178,7 @@ export class ToolbarFixture { } selectBoolean = async (operation: 'union' | 'subtract' | 'intersect') => { await this.page - .getByRole('button', { name: 'caret down Union: open menu' }) + .getByRole('button', { name: 'caret down booleans: open menu' }) .click() const operationTestId = `dropdown-boolean-${operation}` await expect(this.page.getByTestId(operationTestId)).toBeVisible() @@ -186,25 +186,19 @@ export class ToolbarFixture { } selectCircleThreePoint = async () => { - await this.page - .getByRole('button', { name: 'caret down Center circle:' }) - .click() + await this.page.getByRole('button', { name: 'caret down circles:' }).click() await expect( this.page.getByTestId('dropdown-circle-three-points') ).toBeVisible() await this.page.getByTestId('dropdown-circle-three-points').click() } selectArc = async () => { - await this.page - .getByRole('button', { name: 'caret down Tangential Arc:' }) - .click() + await this.page.getByRole('button', { name: 'caret down arcs:' }).click() await expect(this.page.getByTestId('dropdown-arc')).toBeVisible() await this.page.getByTestId('dropdown-arc').click() } selectThreePointArc = async () => { - await this.page - .getByRole('button', { name: 'caret down Tangential Arc:' }) - .click() + await this.page.getByRole('button', { name: 'caret down arcs:' }).click() await expect( this.page.getByTestId('dropdown-three-point-arc') ).toBeVisible() diff --git a/e2e/playwright/testing-constraints.spec.ts b/e2e/playwright/testing-constraints.spec.ts index 12755f752..3882e3e20 100644 --- a/e2e/playwright/testing-constraints.spec.ts +++ b/e2e/playwright/testing-constraints.spec.ts @@ -115,7 +115,7 @@ test.describe('Testing constraints', { tag: ['@skipWin'] }, () => { await page.waitForTimeout(100) // this wait is needed for webkit - not sure why await page .getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) .click() await page.getByRole('button', { name: 'remove constraints' }).click() @@ -189,7 +189,7 @@ test.describe('Testing constraints', { tag: ['@skipWin'] }, () => { await page.waitForTimeout(100) await page .getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) .click() await page @@ -299,7 +299,7 @@ test.describe('Testing constraints', { tag: ['@skipWin'] }, () => { await page.keyboard.up('Shift') await page .getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) .click() await page.getByRole('button', { name: constraint }).click() @@ -420,7 +420,7 @@ test.describe('Testing constraints', { tag: ['@skipWin'] }, () => { await page.waitForTimeout(100) await page .getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) .click() await page @@ -533,7 +533,7 @@ test.describe('Testing constraints', { tag: ['@skipWin'] }, () => { await page.keyboard.up('Shift') await page .getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) .click() await page.getByTestId('dropdown-constraint-angle').click() @@ -627,7 +627,7 @@ test.describe('Testing constraints', { tag: ['@skipWin'] }, () => { await page.mouse.click(line3.x, line3.y) await page .getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) .click() await page.getByTestId('dropdown-constraint-' + constraint).click() @@ -719,7 +719,7 @@ part002 = startSketchOn(XZ) await page.mouse.click(line3.x, line3.y) await page .getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) .click() await page.getByTestId('dropdown-constraint-' + constraint).click() @@ -817,7 +817,7 @@ part002 = startSketchOn(XZ) const activeLinesContent = await page.locator('.cm-activeLine').all() const constraintMenuButton = page.getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) const constraintButton = page .getByRole('button', { @@ -905,7 +905,7 @@ part002 = startSketchOn(XZ) await page.mouse.click(line3.x - 3, line3.y + 20) await page.keyboard.up('Shift') const constraintMenuButton = page.getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) const constraintButton = page.getByRole('button', { name: constraintName, @@ -990,7 +990,7 @@ part002 = startSketchOn(XZ) await page.keyboard.up('Shift') await page.waitForTimeout(100) const constraintMenuButton = page.getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) const constraintButton = page.getByRole('button', { name: constraintName, @@ -1057,7 +1057,7 @@ part002 = startSketchOn(XZ) await page .getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) .click() await page.waitForTimeout(500) diff --git a/e2e/playwright/testing-selections.spec.ts b/e2e/playwright/testing-selections.spec.ts index 1b2485dae..72572f602 100644 --- a/e2e/playwright/testing-selections.spec.ts +++ b/e2e/playwright/testing-selections.spec.ts @@ -124,7 +124,7 @@ test.describe('Testing selections', { tag: ['@skipWin'] }, () => { // click a segment hold shift and click an axis, see that a relevant constraint is enabled const constrainButton = page.getByRole('button', { - name: 'Length: open menu', + name: 'constraints: open menu', }) const absXButton = page.getByRole('button', { name: 'Absolute X' }) diff --git a/src/Toolbar.tsx b/src/Toolbar.tsx index 93a438652..fc9aa65d2 100644 --- a/src/Toolbar.tsx +++ b/src/Toolbar.tsx @@ -17,12 +17,14 @@ import { isDesktop } from '@src/lib/isDesktop' import { openExternalBrowserIfDesktop } from '@src/lib/openWindow' import { editorManager, kclManager } from '@src/lib/singletons' import type { + ToolbarDropdown, ToolbarItem, ToolbarItemCallbackProps, ToolbarItemResolved, + ToolbarItemResolvedDropdown, ToolbarModeName, } from '@src/lib/toolbar' -import { toolbarConfig } from '@src/lib/toolbar' +import { isToolbarItemResolvedDropdown, toolbarConfig } from '@src/lib/toolbar' import { isArray } from '@src/lib/utils' import { commandBarActor } from '@src/machines/commandBarMachine' @@ -131,21 +133,27 @@ export function Toolbar({ */ const currentModeItems: ( | ToolbarItemResolved - | ToolbarItemResolved[] + | ToolbarItemResolvedDropdown | 'break' )[] = useMemo(() => { return toolbarConfig[currentMode].items.map((maybeIconConfig) => { if (maybeIconConfig === 'break') { return 'break' - } else if (isArray(maybeIconConfig)) { - return maybeIconConfig.map(resolveItemConfig) + } else if (isToolbarDropdown(maybeIconConfig)) { + return { + id: maybeIconConfig.id, + array: maybeIconConfig.array.map((item) => + resolveItemConfig(item, maybeIconConfig.id) + ), + } } else { return resolveItemConfig(maybeIconConfig) } }) function resolveItemConfig( - maybeIconConfig: ToolbarItem + maybeIconConfig: ToolbarItem, + dropdownId?: string ): ToolbarItemResolved { const isDisabled = disableAllButtons || @@ -176,6 +184,14 @@ export function Toolbar({ } }, [currentMode, disableAllButtons, configCallbackProps]) + // To remember the last selected item in an ActionButtonDropdown + const [lastSelectedMultiActionItem, _] = useState( + new Map< + number /* index in currentModeItems */, + number /* index in maybeIconConfig */ + >() + ) + return ( ) - } else if (isArray(maybeIconConfig)) { + } else if (isToolbarItemResolvedDropdown(maybeIconConfig)) { // A button with a dropdown + const selectedIcon = + maybeIconConfig.array.find((c) => c.isActive) || + maybeIconConfig.array[lastSelectedMultiActionItem.get(i) ?? 0] + + // Save the last selected item in the dropdown + lastSelectedMultiActionItem.set( + i, + maybeIconConfig.array.indexOf(selectedIcon) + ) return ( ({ + splitMenuItems={maybeIconConfig.array.map((itemConfig) => ({ id: itemConfig.id, label: itemConfig.title, hotkey: itemConfig.hotkey, @@ -236,11 +261,11 @@ export function Toolbar({ > - maybeIconConfig[0].onClick(configCallbackProps) - } + aria-description={selectedIcon.description} + onClick={() => selectedIcon.onClick(configCallbackProps)} > - - {maybeIconConfig[0].title} + + {selectedIcon.title} {showRichContent ? ( ) : ( )} @@ -430,7 +451,9 @@ const ToolbarItemTooltipShortContent = ({ > {title} {hotkey && ( - {hotkey} + + {displayHotkeys(hotkey)} + )} ) @@ -461,7 +484,9 @@ const ToolbarItemTooltipRichContent = ({ {itemConfig.title} {itemConfig.status === 'available' && itemConfig.hotkey ? ( - {itemConfig.hotkey} + + {displayHotkeys(itemConfig.hotkey)} + ) : itemConfig.status === 'kcl-only' ? ( <> @@ -522,3 +547,14 @@ const ToolbarItemTooltipRichContent = ({ ) } + +// We don't want to display Esc hotkeys to avoid confusion in the Toolbar UI (eg. "EscR") +function displayHotkeys(hotkey: string | string[]) { + return (isArray(hotkey) ? hotkey : [hotkey]).filter((h) => h !== 'Esc') +} + +function isToolbarDropdown( + item: ToolbarItem | ToolbarDropdown +): item is ToolbarDropdown { + return 'array' in item +} diff --git a/src/components/ActionButton.tsx b/src/components/ActionButton.tsx index 766d19d93..3da3e0b65 100644 --- a/src/components/ActionButton.tsx +++ b/src/components/ActionButton.tsx @@ -71,6 +71,7 @@ export const ActionButton = forwardRef((props: ActionButtonProps, ref) => {