tests/request/copy-request/copy-folder.spec.ts (2)

9-91: Consider using test.step for better test reporting.

Breaking down each test into logical steps using test.step would improve readability in generated reports. For example:

test('should copy and paste a folder within the same collection', async ({ page, createTmpDir }) => {
  await test.step('Create collection and folder', async () => {
    await createCollection(page, 'test-collection', await createTmpDir('test-collection'), { openWithSandboxMode: 'safe' });
    // ... folder creation logic
  });
  
  await test.step('Add request to folder', async () => {
    // ... request creation logic
  });
  
  await test.step('Copy and paste folder', async () => {
    // ... copy/paste logic
  });
});

As per coding guidelines, test.step usage is promoted for clearer test reports.

---

9-91: Consider extracting more locators into variables.

While some locators are already stored in variables (e.g., collection, folder), frequently used locators like dropdown items and buttons could be extracted for better maintainability:

const newFolderMenuItem = page.locator('.dropdown-item').filter({ hasText: 'New Folder' });
const copyMenuItem = page.locator('.dropdown-item').filter({ hasText: 'Copy' });
const pasteMenuItem = page.locator('.dropdown-item').filter({ hasText: 'Paste' });

tests/request/copy-request/keyboard-shortcuts.spec.ts (3)

4-7: Test isolation concern: second test depends on first test's state.

The folder test (line 53) relies on the collection created by the request test. If the first test fails or is skipped, the second will fail. Consider creating the collection in a beforeAll or beforeEach hook, or making each test independent.

 test.describe('Copy and Paste with Keyboard Shortcuts', () => {
+  test.beforeAll(async ({ page, createTmpDir }) => {
+    await createCollection(page, 'keyboard-test', await createTmpDir('keyboard-test'), { openWithSandboxMode: 'safe' });
+  });
+
   test.afterAll(async ({ page }) => {
     await closeAllCollections(page);
   });

---

9-51: Consider wrapping logical steps with test.step for improved report readability.

Per coding guidelines, using test.step makes generated reports easier to read. This test has clear logical phases that would benefit from this pattern.

   test('should copy and paste request using keyboard shortcuts', async ({ page, createTmpDir }) => {
-    await createCollection(page, 'keyboard-test', await createTmpDir('keyboard-test'), { openWithSandboxMode: 'safe' });
-    const collection = page.locator('.collection-name').filter({ hasText: 'keyboard-test' });
-
-    // Create a request
-    await collection.locator('.collection-actions').hover();
-    ...
+    await test.step('Create collection and request', async () => {
+      await createCollection(page, 'keyboard-test', await createTmpDir('keyboard-test'), { openWithSandboxMode: 'safe' });
+      const collection = page.locator('.collection-name').filter({ hasText: 'keyboard-test' });
+      await collection.locator('.collection-actions').hover();
+      // ... request creation logic
+    });
+
+    await test.step('Copy request using keyboard shortcut', async () => {
+      // ... copy logic
+    });
+
+    await test.step('Paste request using keyboard shortcut', async () => {
+      // ... paste logic
+    });

---

25-30: Redundant .click() before .focus() — click() already focuses the element.

Calling click() followed by focus() is redundant since clicking an element naturally focuses it. This pattern is repeated for both request and folder tests.

     // Focus the request item
     await requestItem.click();
-    await requestItem.focus();

packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js (1)

610-618: Consider using a distinct icon for Copy vs Clone.

Both "Clone" (line 606) and "Copy" (line 615) use IconCopy. This may confuse users about the difference between the two actions.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js (1)

423-438: Extract Mac/modifier-key detection into a shared keyboard utility

The isMac + isModifierPressed logic in handleKeyDown is (or was) also present in Collection/index.js. To avoid duplication and keep platform detection consistent, it’s worth moving this into a small shared helper (e.g., utils/keyboard.js) and using it here:

// utils/keyboard.js
export const isMacPlatform = () =>
  navigator.userAgent?.includes('Mac') || navigator.platform?.startsWith('Mac');

export const isModifierKeyPressed = (e) =>
  isMacPlatform() ? e.metaKey : e.ctrlKey;

Then:

-  const handleKeyDown = (e) => {
-    const isMac = navigator.userAgent?.includes('Mac') || navigator.platform?.startsWith('Mac');
-    const isModifierPressed = isMac ? e.metaKey : e.ctrlKey;
+  const handleKeyDown = (e) => {
+    const isModifierPressed = isModifierKeyPressed(e);

This keeps the keyboard shortcut logic focused on intent (copy/paste) and centralises the platform heuristics.

packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js (1)

398-421: Make dropdown ref usage defensive for keyboard-triggered copy/paste

handleCopyItem / handlePasteItem are now called both from the dropdown and via keyboard shortcuts, but they unconditionally call dropdownTippyRef.current.hide(). In keyboard flows, there’s a risk that the ref isn’t initialised yet, which would throw.

Consider guarding the ref and (optionally) aligning paste behavior with hasCopiedItems:

 const handleCopyItem = () => {
-  dropdownTippyRef.current.hide();
+  dropdownTippyRef.current?.hide();
   dispatch(copyRequest(item));
   const itemType = isFolder ? 'Folder' : 'Request';
   toast.success(`${itemType} copied to clipboard`);
 };

 const handlePasteItem = () => {
-  dropdownTippyRef.current.hide();
+  dropdownTippyRef.current?.hide();
 
   // Only allow paste into folders
   if (!isFolder) {
     toast.error('Paste is only available for folders');
     return;
   }

+  // Optional: mirror menu gating and avoid backend "No item in clipboard" errors
+  // if (!hasCopiedItems) {
+  //   return;
+  // }
+
   dispatch(pasteItem(collectionUid, item.uid))
     .then(() => {
       toast.success('Item pasted successfully');
     })
     .catch((err) => {
       toast.error(err ? err.message : 'An error occurred while pasting the item');
     });
 };

This keeps dropdown behavior unchanged while making the handlers safe for shortcut usage.

Also applies to: 610-618, 622-622

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro