refactor(shortcuts): implement Alt modifier, improve modifier system to handle ctrl/cmd, alt and no-modifier cases, update tests

gustedeveloper · gustedeveloper · commit cd653d8f6950 · 2025-10-26T09:45:21.000+01:00
diff --git a/src/common/components/action-button/action-button.component.spec.tsx b/src/common/components/action-button/action-button.component.spec.tsx
@@ -2,12 +2,27 @@ import { fireEvent, render, screen } from '@testing-library/react';
 import { vi } from 'vitest';
 import { ActionButton } from './action-button.component';
 import { ShortcutOptions } from '@/common/shortcut';
+import { useModalDialogContext } from '@/core/providers';
+
+vi.mock('@/core/providers', () => ({
+  useModalDialogContext: vi.fn(),
+}));
 
 describe('ActionButton', () => {
   let onClick: () => void;
   let shortcutOptions: ShortcutOptions;
 
   beforeEach(() => {
+    vi.mocked(useModalDialogContext).mockReturnValue({
+      modalDialog: {
+        isOpen: false,
+        selectedComponent: null,
+        title: '',
+      },
+      openModal: vi.fn(),
+      closeModal: vi.fn(),
+    });
+
     onClick = vi.fn();
 
     shortcutOptions = {
@@ -52,19 +67,84 @@ describe('ActionButton', () => {
     expect(onClick).toHaveBeenCalled();
   });
 
-  it('should render the tooltip with the correct shortcut key', () => {
-    const { getByRole } = render(
-      <ActionButton
-        icon={<span>Icon</span>}
-        label="Label"
-        onClick={onClick}
-        shortcutOptions={shortcutOptions}
-      />
-    );
+  describe('tooltip display', () => {
+    it('should render system modifier correctly', () => {
+      // Test Mac
+      const { getByRole, rerender } = render(
+        <ActionButton
+          icon={<span>Icon</span>}
+          label="Label"
+          onClick={onClick}
+          shortcutOptions={{
+            ...shortcutOptions,
+            modifierType: 'system',
+          }}
+        />
+      );
+      expect(getByRole('tooltip').textContent).toBe('(⌘ + A)');
+
+      // Test Windows
+      Object.defineProperty(window.navigator, 'userAgent', {
+        value: 'Windows',
+        configurable: true,
+      });
+      rerender(
+        <ActionButton
+          icon={<span>Icon</span>}
+          label="Label"
+          onClick={onClick}
+          shortcutOptions={{
+            ...shortcutOptions,
+            modifierType: 'system',
+          }}
+        />
+      );
+      expect(getByRole('tooltip').textContent).toBe('(Ctrl + A)');
+    });
 
-    const tooltip = getByRole('tooltip');
+    it('should render alt tooltip correctly', () => {
+      const { getByRole } = render(
+        <ActionButton
+          icon={<span>Icon</span>}
+          label="Label"
+          onClick={onClick}
+          shortcutOptions={{
+            ...shortcutOptions,
+            modifierType: 'alt',
+          }}
+        />
+      );
+      expect(getByRole('tooltip').textContent).toBe('(Alt + A)');
+    });
+
+    it('should render mo-modifier tooltip correctly', () => {
+      const { getByRole } = render(
+        <ActionButton
+          icon={<span>Icon</span>}
+          label="Label"
+          onClick={onClick}
+          shortcutOptions={{
+            ...shortcutOptions,
+            modifierType: 'none',
+          }}
+        />
+      );
+      expect(getByRole('tooltip').textContent).toBe('(A)');
+    });
 
-    expect(tooltip.textContent).toContain('⌘ + A');
+    it('should not show tooltip when disabled', () => {
+      const { queryByRole } = render(
+        <ActionButton
+          icon={<span>Icon</span>}
+          label="Label"
+          onClick={onClick}
+          disabled
+          shortcutOptions={shortcutOptions}
+        />
+      );
+
+      expect(queryByRole('tooltip')).toBeNull();
+    });
   });
 
   it('should disable the button if the disabled prop is true', () => {
@@ -125,23 +205,4 @@ describe('ActionButton', () => {
     const tooltip = getByRole('tooltip');
     expect(tooltip.className).toContain('tooltipTop');
   });
-
-  it('should render the tooltip without modifier when noModifier is true', () => {
-    const shortcutOptionsNoMod = {
-      ...shortcutOptions,
-      noModifier: true,
-    };
-
-    const { getByRole } = render(
-      <ActionButton
-        icon={<span>Icon</span>}
-        label="Label"
-        onClick={onClick}
-        shortcutOptions={shortcutOptionsNoMod}
-      />
-    );
-
-    const tooltip = getByRole('tooltip');
-    expect(tooltip.textContent).toBe('A');
-  });
 });
diff --git a/src/common/components/action-button/action-button.component.tsx b/src/common/components/action-button/action-button.component.tsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import { isMacOS } from '@/common/helpers/platform.helpers';
 import classes from './action-button.component.module.css';
-import { ShortcutOptions } from '@/common/shortcut';
+import { ModifierType, ShortcutOptions } from '@/common/shortcut';
 import useShortcut from '@/common/shortcut/shortcut.hook';
 
 interface Props {
@@ -25,11 +25,28 @@ export const ActionButton: React.FC<Props> = ({
   showLabel = true,
   tooltipPosition = 'bottom',
 }) => {
-  const shortcutCommand = isMacOS() ? '⌘' : 'Ctrl';
+  const getModifierSymbol = (modifierType: ModifierType = 'system') => {
+    switch (modifierType) {
+      case 'none':
+        return '';
+      case 'alt':
+        return 'Alt';
+      case 'system':
+      default:
+        return isMacOS() ? '⌘' : 'Ctrl';
+    }
+  };
+
+  const shortcutCommand = getModifierSymbol(shortcutOptions?.modifierType);
+
   const showTooltip = shortcutOptions && !disabled;
-  const tooltipText = shortcutOptions?.noModifier
-    ? `${shortcutOptions.targetKeyLabel}`
-    : `(${shortcutCommand} + ${shortcutOptions?.targetKeyLabel})`;
+  const tooltipText =
+    shortcutOptions &&
+    `(${
+      shortcutOptions.modifierType === 'none'
+        ? shortcutOptions.targetKeyLabel
+        : `${shortcutCommand} + ${shortcutOptions.targetKeyLabel}`
+    })`;
 
   const tooltipPositionClass =
     tooltipPosition === 'top' ? classes.tooltipTop : classes.tooltipBottom;
diff --git a/src/common/shortcut/shortcut.const.ts b/src/common/shortcut/shortcut.const.ts
@@ -10,21 +10,21 @@ export const SHORTCUTS: Shortcut = {
     id: 'add-collection-button-shortcut',
     targetKey: ['c'],
     targetKeyLabel: 'Collection "C"',
-    noModifier: true,
+    modifierType: 'none',
   },
   addRelation: {
     description: 'Add Relation',
     id: 'add-relation-button-shortcut',
     targetKey: ['r'],
     targetKeyLabel: 'Relation "R"',
-    noModifier: true,
+    modifierType: 'none',
   },
   delete: {
     description: 'Delete',
     id: 'delete-button-shortcut',
     targetKey: ['backspace'],
     targetKeyLabel: 'Backspace',
-    noModifier: true,
+    modifierType: 'none',
   },
   export: {
     description: 'Export',
@@ -37,6 +37,7 @@ export const SHORTCUTS: Shortcut = {
     id: 'new-button-shortcut',
     targetKey: ['n'],
     targetKeyLabel: 'N',
+    modifierType: 'alt',
   },
   open: {
     description: 'Open',
diff --git a/src/common/shortcut/shortcut.hook.spec.tsx b/src/common/shortcut/shortcut.hook.spec.tsx
@@ -1,12 +1,27 @@
 import { renderHook } from '@testing-library/react';
 import useShortcut from './shortcut.hook';
 import { vi } from 'vitest';
+import { useModalDialogContext } from '@/core/providers';
+
+vi.mock('@/core/providers', () => ({
+  useModalDialogContext: vi.fn(),
+}));
 
 describe('useShortcut', () => {
   let targetKey: string[];
   let callback: () => void;
 
   beforeEach(() => {
+    vi.mocked(useModalDialogContext).mockReturnValue({
+      modalDialog: {
+        isOpen: false,
+        selectedComponent: null,
+        title: '',
+      },
+      openModal: vi.fn(),
+      closeModal: vi.fn(),
+    });
+
     targetKey = ['a'];
     callback = vi.fn();
 
@@ -110,51 +125,51 @@ describe('useShortcut', () => {
     expect(callback).not.toHaveBeenCalled();
   });
 
-  it('should call the callback when noModifier is true and only the key is pressed', () => {
+  it('should call callback with alt modifier', () => {
     renderHook(() =>
       useShortcut({
         targetKey,
         callback,
-        noModifier: true,
+        modifierType: 'alt',
       })
     );
 
     const event = new KeyboardEvent('keydown', {
       key: 'a',
       code: 'KeyA',
+      altKey: true,
     });
 
     window.dispatchEvent(event);
-
     expect(callback).toHaveBeenCalled();
   });
 
-  it('should not call the callback when noModifier is true and modifier is pressed', () => {
+  it('should not call callback if other modifiers are pressed with alt', () => {
     renderHook(() =>
       useShortcut({
         targetKey,
         callback,
-        noModifier: true,
+        modifierType: 'alt',
       })
     );
 
     const event = new KeyboardEvent('keydown', {
       key: 'a',
       code: 'KeyA',
-      ctrlKey: true,
+      altKey: true,
+      ctrlKey: true, // No debería funcionar con otros modificadores
     });
 
     window.dispatchEvent(event);
-
     expect(callback).not.toHaveBeenCalled();
   });
 
-  it('should not call the callback when noModifier is false and no modifier is pressed', () => {
+  it('should call callback with no modifiers', () => {
     renderHook(() =>
       useShortcut({
         targetKey,
         callback,
-        noModifier: false,
+        modifierType: 'none',
       })
     );
 
@@ -164,7 +179,53 @@ describe('useShortcut', () => {
     });
 
     window.dispatchEvent(event);
+    expect(callback).toHaveBeenCalled();
+  });
 
+  it('should not call callback if any modifier is pressed', () => {
+    renderHook(() =>
+      useShortcut({
+        targetKey,
+        callback,
+        modifierType: 'none',
+      })
+    );
+
+    const event = new KeyboardEvent('keydown', {
+      key: 'a',
+      code: 'KeyA',
+      altKey: true,
+    });
+
+    window.dispatchEvent(event);
+    expect(callback).not.toHaveBeenCalled();
+  });
+
+  it('should not call callback when modal is open', () => {
+    vi.mocked(useModalDialogContext).mockReturnValueOnce({
+      modalDialog: {
+        isOpen: true,
+        selectedComponent: null,
+        title: '',
+      },
+      openModal: vi.fn(),
+      closeModal: vi.fn(),
+    });
+
+    renderHook(() =>
+      useShortcut({
+        targetKey,
+        callback,
+        modifierType: 'none',
+      })
+    );
+
+    const event = new KeyboardEvent('keydown', {
+      key: 'r',
+      code: 'KeyR',
+    });
+
+    window.dispatchEvent(event);
     expect(callback).not.toHaveBeenCalled();
   });
 });
diff --git a/src/common/shortcut/shortcut.hook.tsx b/src/common/shortcut/shortcut.hook.tsx
diff --git a/src/common/shortcut/shortcut.model.ts b/src/common/shortcut/shortcut.model.ts
