bump three point arc to be the default (#6464)

* bump three point arc up * Update tests to select tangential arc from dropdown * Fix another test that used tangential arc * Fix up snapshot tests' use of Tangential Arc --------- Co-authored-by: Frank Noirot <frankjohnson1993@gmail.com>
2025-04-25 06:40:33 +10:00
parent 6e0f1e71b2
commit 6641e1178b
5 changed files with 70 additions and 69 deletions
--- a/e2e/playwright/fixtures/toolbarFixture.ts
+++ b/e2e/playwright/fixtures/toolbarFixture.ts
@ -174,6 +174,13 @@ export class ToolbarFixture {
  openFile = async (fileName: string) => {
    await this.filePane.getByText(fileName).click()
  }
+  selectTangentialArc = async () => {
+    await this.page.getByRole('button', { name: 'caret down arcs:' }).click()
+    await expect(
+      this.page.getByTestId('dropdown-three-point-arc')
+    ).toBeVisible()
+    await this.page.getByTestId('dropdown-tangential-arc').click()
+  }
  selectCenterRectangle = async () => {
    await this.page
      .getByRole('button', { name: 'caret down rectangles:' })
--- a/e2e/playwright/sketch-tests.spec.ts
+++ b/e2e/playwright/sketch-tests.spec.ts
@ -1564,7 +1564,7 @@ profile003 = startProfileAt([206.63, -56.73], sketch001)

      const codeFromTangentialArc = `  |> tangentialArc(endAbsolute = [39.49, 88.22])`
      await test.step('check that tangential tool does not snap to other profile starts', async () => {
-        await toolbar.tangentialArcBtn.click()
+        await toolbar.selectTangentialArc()
        await page.waitForTimeout(1000)
        await endOfLowerSegMove()
        await page.waitForTimeout(1000)
@ -1777,7 +1777,7 @@ profile003 = startProfileAt([206.63, -56.73], sketch001)

      await endLineStartTanArc()
      await editor.expectEditor.toContain(`|> line(end = [9.02, -0.55])`)
-      await toolbar.tangentialArcBtn.click()
+      await toolbar.selectTangentialArc()
      await page.waitForTimeout(300)
      await page.mouse.click(745, 359)
      await page.waitForTimeout(300)
@ -2970,7 +2970,7 @@ test.describe('Redirecting to home page and back to the original file should cle
    await click00r(200, -200)

    // Draw arc
-    await toolbar.tangentialArcBtn.click()
+    await toolbar.selectTangentialArc()
    await click00r(0, 0)
    await click00r(100, 100)

--- a/e2e/playwright/snapshot-tests.spec.ts
+++ b/e2e/playwright/snapshot-tests.spec.ts
@ -464,9 +464,7 @@ test(
  |> xLine(length = 184.3)`
    await expect(page.locator('.cm-content')).toHaveText(code)

-    await page
-      .getByRole('button', { name: 'arc Tangential Arc', exact: true })
-      .click()
+    await toolbar.selectTangentialArc()

    // click on the end of the profile to continue it
    await page.waitForTimeout(500)
@ -621,7 +619,7 @@ test.describe(
  'Client side scene scale should match engine scale',
  { tag: '@snapshot' },
  () => {
-    test('Inch scale', async ({ page, cmdBar, scene }) => {
+    test('Inch scale', async ({ page, cmdBar, scene, toolbar }) => {
      const u = await getUtils(page)
      await page.setViewportSize({ width: 1200, height: 500 })
      const PUR = 400 / 37.5 //pixeltoUnitRatio
@ -655,9 +653,7 @@ test.describe(
  |> xLine(length = 184.3)`
      await expect(u.codeLocator).toHaveText(code)

-      await page
-        .getByRole('button', { name: 'arc Tangential Arc', exact: true })
-        .click()
+      await toolbar.selectTangentialArc()
      await page.waitForTimeout(100)

      // click to continue profile
@ -671,9 +667,8 @@ test.describe(
      await expect(u.codeLocator).toHaveText(code)

      // click tangential arc tool again to unequip it
-      await page
-        .getByRole('button', { name: 'arc Tangential Arc', exact: true })
-        .click()
+      // it will be available directly in the toolbar since it was last equipped
+      await toolbar.tangentialArcBtn.click()
      await page.waitForTimeout(100)

      // screen shot should show the sketch
@ -696,7 +691,13 @@ test.describe(
      })
    })

-    test('Millimeter scale', async ({ page, context, cmdBar, scene }) => {
+    test('Millimeter scale', async ({
+      page,
+      context,
+      cmdBar,
+      scene,
+      toolbar,
+    }) => {
      await context.addInitScript(
        async ({ settingsKey, settings }) => {
          localStorage.setItem(settingsKey, settings)
@ -749,9 +750,7 @@ test.describe(
  |> xLine(length = 184.3)`
      await expect(u.codeLocator).toHaveText(code)

-      await page
-        .getByRole('button', { name: 'arc Tangential Arc', exact: true })
-        .click()
+      await toolbar.selectTangentialArc()
      await page.waitForTimeout(100)

      // click to continue profile
@ -764,9 +763,7 @@ test.describe(
  |> tangentialArc(endAbsolute = [551.2, -62.01])`
      await expect(u.codeLocator).toHaveText(code)

-      await page
-        .getByRole('button', { name: 'arc Tangential Arc', exact: true })
-        .click()
+      await toolbar.tangentialArcBtn.click()
      await page.waitForTimeout(100)

      // screen shot should show the sketch
--- a/e2e/playwright/various.spec.ts
+++ b/e2e/playwright/various.spec.ts
@ -178,6 +178,7 @@ test('Keyboard shortcuts can be viewed through the help menu', async ({
 test('First escape in tool pops you out of tool, second exits sketch mode', async ({
  page,
  homePage,
+  toolbar,
 }) => {
  // Wait for the app to be ready for use
  const u = await getUtils(page)
@ -188,15 +189,6 @@ test('First escape in tool pops you out of tool, second exits sketch mode', asyn
  await u.expectCmdLog('[data-message-type="execution-done"]')
  await u.closeDebugPanel()

-  const lineButton = page.getByRole('button', {
-    name: 'line Line',
-    exact: true,
-  })
-  const arcButton = page.getByRole('button', {
-    name: 'arc Tangential Arc',
-    exact: true,
-  })
-
  // Test these hotkeys perform actions when
  // focus is on the canvas
  await page.mouse.move(600, 250)
@ -207,8 +199,8 @@ test('First escape in tool pops you out of tool, second exits sketch mode', asyn
  await page.mouse.move(800, 300)
  await page.mouse.click(800, 300)
  await page.waitForTimeout(1000)
-  await expect(lineButton).toBeVisible()
-  await expect(lineButton).toHaveAttribute('aria-pressed', 'true')
+  await expect(toolbar.lineBtn).toBeVisible()
+  await expect(toolbar.lineBtn).toHaveAttribute('aria-pressed', 'true')

  // Draw a line
  await page.mouse.move(700, 200, { steps: 5 })
@ -224,10 +216,9 @@ test('First escape in tool pops you out of tool, second exits sketch mode', asyn
  await page.keyboard.press('Escape')
  // Make sure we didn't pop out of sketch mode.
  await expect(page.getByRole('button', { name: 'Exit Sketch' })).toBeVisible()
-  await expect(lineButton).not.toHaveAttribute('aria-pressed', 'true')
+  await expect(toolbar.lineBtn).not.toHaveAttribute('aria-pressed', 'true')
  // Equip arc tool
-  await page.keyboard.press('a')
-  await expect(arcButton).toHaveAttribute('aria-pressed', 'true')
+  await toolbar.selectTangentialArc()

  // click in the same position again to continue the profile
  await page.mouse.move(secondMousePosition.x, secondMousePosition.y, {
@ -238,11 +229,14 @@ test('First escape in tool pops you out of tool, second exits sketch mode', asyn
  await page.mouse.move(1000, 100, { steps: 5 })
  await page.mouse.click(1000, 100)
  await page.keyboard.press('Escape')
-  await expect(arcButton).toHaveAttribute('aria-pressed', 'false')
+  await expect(toolbar.tangentialArcBtn).toHaveAttribute(
+    'aria-pressed',
+    'false'
+  )
  await expect
    .poll(async () => {
      await page.keyboard.press('l')
-      return lineButton.getAttribute('aria-pressed')
+      return toolbar.lineBtn.getAttribute('aria-pressed')
    })
    .toBe('true')

@ -251,8 +245,11 @@ test('First escape in tool pops you out of tool, second exits sketch mode', asyn

  // Unequip line tool
  await page.keyboard.press('Escape')
-  await expect(lineButton).toHaveAttribute('aria-pressed', 'false')
-  await expect(arcButton).toHaveAttribute('aria-pressed', 'false')
+  await expect(toolbar.lineBtn).toHaveAttribute('aria-pressed', 'false')
+  await expect(toolbar.tangentialArcBtn).toHaveAttribute(
+    'aria-pressed',
+    'false'
+  )
  // Make sure we didn't pop out of sketch mode.
  await expect(page.getByRole('button', { name: 'Exit Sketch' })).toBeVisible()
  // Exit sketch
--- a/src/lib/toolbar.ts
+++ b/src/lib/toolbar.ts
@ -492,37 +492,6 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
      {
        id: 'arcs',
        array: [
-          {
-            id: 'tangential-arc',
-            onClick: ({ modelingState, modelingSend }) =>
-              modelingSend({
-                type: 'change tool',
-                data: {
-                  tool: !modelingState.matches({ Sketch: 'Tangential arc to' })
-                    ? 'tangentialArc'
-                    : 'none',
-                },
-              }),
-            icon: 'arc',
-            status: 'available',
-            disabled: (state) =>
-              (!isEditingExistingSketch(state.context) &&
-                !state.matches({ Sketch: 'Tangential arc to' })) ||
-              pipeHasCircle(state.context),
-            disabledReason: (state) =>
-              !isEditingExistingSketch(state.context) &&
-              !state.matches({ Sketch: 'Tangential arc to' })
-                ? "Cannot start a tangential arc because there's no previous line to be tangential to.  Try drawing a line first or selecting an existing sketch to edit."
-                : undefined,
-            title: 'Tangential Arc',
-            hotkey: (state) =>
-              state.matches({ Sketch: 'Tangential arc to' })
-                ? ['Esc', 'A']
-                : 'A',
-            description: 'Start drawing an arc tangent to the current segment',
-            links: [],
-            isActive: (state) => state.matches({ Sketch: 'Tangential arc to' }),
-          },
          {
            id: 'three-point-arc',
            onClick: ({ modelingState, modelingSend }) =>
@ -554,6 +523,37 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
            isActive: (state) =>
              state.matches({ Sketch: 'Arc three point tool' }),
          },
+          {
+            id: 'tangential-arc',
+            onClick: ({ modelingState, modelingSend }) =>
+              modelingSend({
+                type: 'change tool',
+                data: {
+                  tool: !modelingState.matches({ Sketch: 'Tangential arc to' })
+                    ? 'tangentialArc'
+                    : 'none',
+                },
+              }),
+            icon: 'arc',
+            status: 'available',
+            disabled: (state) =>
+              (!isEditingExistingSketch(state.context) &&
+                !state.matches({ Sketch: 'Tangential arc to' })) ||
+              pipeHasCircle(state.context),
+            disabledReason: (state) =>
+              !isEditingExistingSketch(state.context) &&
+              !state.matches({ Sketch: 'Tangential arc to' })
+                ? "Cannot start a tangential arc because there's no previous line to be tangential to.  Try drawing a line first or selecting an existing sketch to edit."
+                : undefined,
+            title: 'Tangential Arc',
+            hotkey: (state) =>
+              state.matches({ Sketch: 'Tangential arc to' })
+                ? ['Esc', 'A']
+                : 'A',
+            description: 'Start drawing an arc tangent to the current segment',
+            links: [],
+            isActive: (state) => state.matches({ Sketch: 'Tangential arc to' }),
+          },
          {
            id: 'arc',
            onClick: ({ modelingState, modelingSend }) =>