Enable hotkeys for toolbar items within dropdowns (#6765)

* Remove gnarly fake union hotkeys * Enable hotkey for items buried in ActionButtonDropdown I'm kinda over `useHotkeys` as a hook * Add hotkeys for other sketch tools * Fix lint and tsc * Fix duplicate locator * The circular dependecies got reordered somehow * Update src/lib/toolbar.ts
2025-05-09 18:56:11 -04:00
parent 2d9f6c7b2a
commit 3e24e2c9e8
6 changed files with 112 additions and 71 deletions
--- a/e2e/playwright/testing-segment-overlays.spec.ts
+++ b/e2e/playwright/testing-segment-overlays.spec.ts
@ -1445,7 +1445,7 @@ part001 = startSketchOn(XZ)

        await page.getByTestId('overlay-menu').click()
        await page.waitForTimeout(100)
-        await page.getByText('Remove constraints').click()
+        await page.getByRole('button', { name: 'Remove constraints' }).click()

        await editor.expectEditor.toContain(after, { shouldNormalise: true })

--- a/known-circular.txt
+++ b/known-circular.txt
@ -3,11 +3,11 @@
 > dpdm --no-warning --no-tree -T --skip-dynamic-imports=circular src/index.tsx

 • Circular Dependencies
-  1) src/lang/std/sketch.ts -> src/lang/modifyAst.ts
-  2) src/lang/std/sketch.ts -> src/lang/modifyAst.ts -> src/lang/std/sketchcombos.ts
-  3) src/hooks/useModelingContext.ts -> src/components/ModelingMachineProvider.tsx -> src/components/Toolbar/setAngleLength.tsx -> src/components/SetAngleLengthModal.tsx -> src/lib/useCalculateKclExpression.ts
-  4) src/lib/singletons.ts -> src/editor/manager.ts -> src/lib/selections.ts
-  5) src/lib/singletons.ts -> src/editor/manager.ts -> src/lib/selections.ts
-  6) src/lib/singletons.ts -> src/lang/codeManager.ts
-  7) src/lib/singletons.ts -> src/clientSideScene/sceneEntities.ts -> src/clientSideScene/segments.ts -> src/components/Toolbar/angleLengthInfo.ts
-  8) src/lib/singletons.ts -> src/clientSideScene/sceneEntities.ts -> src/clientSideScene/segments.ts
+  1) src/lib/singletons.ts -> src/editor/manager.ts -> src/lib/selections.ts
+  2) src/lib/singletons.ts -> src/editor/manager.ts -> src/lib/selections.ts
+  3) src/lib/singletons.ts -> src/lang/codeManager.ts
+  4) src/lang/std/sketch.ts -> src/lang/modifyAst.ts
+  5) src/lang/std/sketch.ts -> src/lang/modifyAst.ts -> src/lang/std/sketchcombos.ts
+  6) src/lib/singletons.ts -> src/clientSideScene/sceneEntities.ts -> src/clientSideScene/segments.ts -> src/components/Toolbar/angleLengthInfo.ts
+  7) src/lib/singletons.ts -> src/clientSideScene/sceneEntities.ts -> src/clientSideScene/segments.ts
+  8) src/hooks/useModelingContext.ts -> src/components/ModelingMachineProvider.tsx -> src/components/Toolbar/setAngleLength.tsx -> src/components/SetAngleLengthModal.tsx -> src/lib/useCalculateKclExpression.ts
--- a/src/Toolbar.tsx
+++ b/src/Toolbar.tsx
@ -25,8 +25,8 @@ import type {
  ToolbarModeName,
 } from '@src/lib/toolbar'
 import { isToolbarItemResolvedDropdown, toolbarConfig } from '@src/lib/toolbar'
-import { isArray } from '@src/lib/utils'
 import { commandBarActor } from '@src/lib/singletons'
+import { filterEscHotkey } from '@src/lib/hotkeyWrapper'

 export function Toolbar({
  className = '',
@ -253,7 +253,8 @@ export function Toolbar({
                    !['available', 'experimental'].includes(
                      itemConfig.status
                    ) ||
-                    itemConfig.disabled === true,
+                    itemConfig.disabled === true ||
+                    itemConfig.disableHotkey === true,
                  status: itemConfig.status,
                }))}
              >
@ -410,6 +411,10 @@ const ToolbarItemTooltip = memo(function ToolbarItemContents({
  contentClassName = '',
  children,
 }: ToolbarItemContentsProps) {
+  /**
+   * GOTCHA: `useHotkeys` can only register one hotkey listener per component.
+   * TODO: make a global hotkey registration system. make them editable.
+   */
  useHotkeys(
    itemConfig.hotkey || '',
    () => {
@ -469,7 +474,7 @@ const ToolbarItemTooltipShortContent = ({
      {title}
      {hotkey && (
        <kbd className="inline-block ml-2 flex-none hotkey">
-          {displayHotkeys(hotkey)}
+          {filterEscHotkey(hotkey)}
        </kbd>
      )}
    </div>
@ -510,7 +515,7 @@ const ToolbarItemTooltipRichContent = ({
        </div>
        {shouldBeEnabled && itemConfig.hotkey ? (
          <kbd className="flex-none hotkey">
-            {displayHotkeys(itemConfig.hotkey)}
+            {filterEscHotkey(itemConfig.hotkey)}
          </kbd>
        ) : itemConfig.status === 'kcl-only' ? (
          <>
@ -573,11 +578,6 @@ 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 {
--- a/src/components/ActionButtonDropdown.tsx
+++ b/src/components/ActionButtonDropdown.tsx
@ -3,6 +3,8 @@ import { Popover } from '@headlessui/react'
 import type { ActionButtonProps } from '@src/components/ActionButton'
 import { CustomIcon } from '@src/components/CustomIcon'
 import Tooltip from '@src/components/Tooltip'
+import { filterEscHotkey } from '@src/lib/hotkeyWrapper'
+import { useHotkeys } from 'react-hotkeys-hook'

 type ActionButtonSplitProps = ActionButtonProps & { Element: 'button' } & {
  name?: string
@ -10,7 +12,7 @@ type ActionButtonSplitProps = ActionButtonProps & { Element: 'button' } & {
  splitMenuItems: {
    id: string
    label: string
-    shortcut?: string
+    hotkey?: string | string[]
    onClick: () => void
    disabled?: boolean
    status?: 'available' | 'unavailable' | 'kcl-only' | 'experimental'
@ -63,24 +65,58 @@ export function ActionButtonDropdown({
          <Popover.Panel
            as="ul"
            className="!pointer-events-auto absolute z-20 left-1/2 -translate-x-1/2 top-full mt-4 w-fit max-w-[280px] max-h-[80vh] overflow-y-auto py-2 flex flex-col align-stretch text-inherit dark:text-chalkboard-10 bg-chalkboard-10 dark:bg-chalkboard-100 rounded shadow-lg border border-solid border-chalkboard-30 dark:border-chalkboard-80 text-sm m-0 p-0"
+            unmount={false}
          >
-            {splitMenuItems.map((item) => (
-              <li className="contents" key={item.label}>
-                <button
+            {splitMenuItems.map((item, index) => (
+              <ActionButtonDropdownListItem
+                item={item}
                onClick={() => {
                  item.onClick()
                  // Close the popover
                  close()
                }}
+                key={item.label}
+              />
+            ))}
+          </Popover.Panel>
+        </>
+      )}
+    </Popover>
+  )
+}
+
+function ActionButtonDropdownListItem({
+  item,
+  onClick,
+}: {
+  item: ActionButtonSplitProps['splitMenuItems'][number]
+  onClick: () => void
+}) {
+  /**
+   * GOTCHA: `useHotkeys` can only register one hotkey listener per component.
+   * and since the first item in the dropdown has a top-level button too,
+   * it already has a hotkey listener, so we should skip it.
+   * TODO: make a global hotkey registration system. make them editable.
+   */
+  useHotkeys(item.hotkey || '', item.onClick, {
+    enabled:
+      ['available', 'experimental'].includes(item.status || '') &&
+      !!item.hotkey &&
+      !item.disabled,
+  })
+
+  return (
+    <li className="contents">
+      <button
+        type="button"
+        onClick={onClick}
        className="group/button flex items-center gap-6 px-3 py-1 font-sans text-xs hover:bg-primary/10 dark:hover:bg-chalkboard-80 border-0 m-0 w-full rounded-none text-left disabled:!bg-transparent dark:disabled:text-chalkboard-60"
        tabIndex={-1}
        disabled={item.disabled}
        data-testid={'dropdown-' + item.id}
-                  data-onboarding-id={`${props.name}-dropdown-item`}
+        data-onboarding-id={`${item.id}-dropdown-item`}
      >
-                  <span className="capitalize flex-grow text-left">
-                    {item.label}
-                  </span>
+        <span className="capitalize flex-grow text-left">{item.label}</span>
        {item.status === 'unavailable' ? (
          <div className="flex flex-none items-center gap-1">
            <span className="text-chalkboard-70 dark:text-chalkboard-40">
@ -101,9 +137,9 @@ export function ActionButtonDropdown({
              className="w-4 h-4 text-chalkboard-70 dark:text-chalkboard-40"
            />
          </div>
-                  ) : item.shortcut ? (
+        ) : item.hotkey ? (
          <kbd className="hotkey flex-none group-disabled/button:text-chalkboard-50 dark:group-disabled/button:text-chalkboard-70 group-disabled/button:border-chalkboard-20 dark:group-disabled/button:border-chalkboard-80">
-                      {item.shortcut}
+            {filterEscHotkey(item.hotkey)}
          </kbd>
        ) : null}
        {item.status === 'experimental' ? (
@ -111,10 +147,5 @@ export function ActionButtonDropdown({
        ) : null}
      </button>
    </li>
-            ))}
-          </Popover.Panel>
-        </>
-      )}
-    </Popover>
  )
 }
--- a/src/lib/hotkeyWrapper.ts
+++ b/src/lib/hotkeyWrapper.ts
@ -3,7 +3,7 @@ import type { Options } from 'react-hotkeys-hook'
 import { useHotkeys } from 'react-hotkeys-hook'

 import { codeManager } from '@src/lib/singletons'
-import type { Platform } from '@src/lib/utils'
+import { isArray, type Platform } from '@src/lib/utils'

 // Hotkey wrapper wraps hotkeys for the app (outside of the editor)
 // with hotkeys inside the editor.
@ -86,3 +86,10 @@ export function hotkeyDisplay(hotkey: string, platform: Platform): string {

  return display
 }
+
+/**
+ * We don't want to display Esc hotkeys to avoid confusion in the Toolbar UI (eg. "EscR")
+ */
+export function filterEscHotkey(hotkey: string | string[]) {
+  return (isArray(hotkey) ? hotkey : [hotkey]).filter((h) => h !== 'Esc')
+}
--- a/src/lib/toolbar.ts
+++ b/src/lib/toolbar.ts
@ -238,7 +238,6 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
            icon: 'booleanUnion',
            status: 'available',
            title: 'Union',
-            hotkey: 'Shift + B U',
            description: 'Combine two or more solids into a single solid.',
            links: [
              {
@ -257,7 +256,6 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
            icon: 'booleanSubtract',
            status: 'available',
            title: 'Subtract',
-            hotkey: 'Shift + B S',
            description: 'Subtract one solid from another.',
            links: [
              {
@ -276,7 +274,6 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
            icon: 'booleanIntersect',
            status: 'available',
            title: 'Intersect',
-            hotkey: 'Shift + B I',
            description: 'Create a solid from the intersection of two solids.',
            links: [
              {
@ -631,7 +628,9 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
            isActive: (state) =>
              state.matches({ Sketch: 'Circle three point tool' }),
            hotkey: (state) =>
-              state.matches({ Sketch: 'Circle three point tool' }) ? 'Esc' : [],
+              state.matches({ Sketch: 'Circle three point tool' })
+                ? ['Alt+C', 'Esc']
+                : 'Alt+C',
            showTitle: false,
            description: 'Draw a circle defined by three points',
            links: [],
@ -681,6 +680,10 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
            title: 'Center rectangle',
            description: 'Start drawing a rectangle from its center',
            links: [],
+            hotkey: (state) =>
+              state.matches({ Sketch: 'Center Rectangle tool' })
+                ? ['Alt+R', 'Esc']
+                : 'Alt+R',
            isActive: (state) => {
              return state.matches({ Sketch: 'Center Rectangle tool' })
            },