packages/bruno-app/src/ui/MenuDropdown/index.js (1)

435-467: Consider potential duplicate global style injections.

Each MenuDropdown instance now injects MenuDropdownGlobalStyle into the document. If multiple MenuDropdowns are rendered simultaneously (e.g., nested menus or multiple tabs with context menus), the global style will be injected multiple times. While styled-components deduplicates identical global styles, this is still unnecessary overhead.

Consider one of these approaches:

1. Inject MenuDropdownGlobalStyle once at the app root level instead of per-component instance
2. If per-instance injection is intentional, add a comment explaining why

🔎 Example: Move global style to app root

In your app root (e.g., App.js or index.js):

+import { MenuDropdownGlobalStyle } from 'ui/MenuDropdown/StyledWrapper';

 function App() {
   return (
     <ThemeProvider theme={theme}>
+      <MenuDropdownGlobalStyle />
       <AppContent />
     </ThemeProvider>
   );
 }

Then remove the global style from MenuDropdown/index.js:

-import { MenuDropdownGlobalStyle } from './StyledWrapper';

 return (
-  <Fragment>
-    <MenuDropdownGlobalStyle />
     <Dropdown
       onCreate={onDropdownCreate}
       // ...
     </Dropdown>
-  </Fragment>
 );

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (3)

427-427: Remove unused tabItem prop.

The tabItem prop is passed from the parent (line 406) but is not destructured or used in the RequestTabMenu function. The component derives currentTabItem independently using findItemInCollection.

🔎 Apply this diff to clean up the unused prop:

In the parent component:

 <RequestTabMenu
   menuDropdownRef={menuDropdownRef}
   tabLabelRef={tabLabelRef}
   tabIndex={tabIndex}
   collectionRequestTabs={collectionRequestTabs}
-  tabItem={item}
   collection={collection}
   dispatch={dispatch}
 />

---

456-470: Empty catch block silently swallows errors.

Similar to handleCloseTab, handleRevertChanges has an empty catch block. Consider adding error logging or user notification.

🔎 Apply this diff:

     try {
       const item = findItemInCollection(collection, currentTabUid);
       if (item.draft) {
         dispatch(deleteRequestDraft({
           itemUid: item.uid,
           collectionUid: collection.uid
         }));
       }
-    } catch (err) { }
+    } catch (err) {
+      console.error('Failed to revert changes:', err);
+      toast.error('Failed to revert changes');
+    }

---

550-567: LGTM! MenuDropdown is correctly configured with ref-based control.

The dropdown is properly set up with document.body rendering and dynamic positioning via getReferenceClientRect. The empty <span> child is a necessary workaround since MenuDropdown requires a trigger element even when controlled via ref.

Consider adding a comment explaining this:

{/* Empty span required for MenuDropdown's trigger prop, actual control is via ref */}
<span></span>

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors

Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)

471-495: forEach with async handleCloseTab doesn't await completion.

This fires all close operations concurrently without waiting, potentially causing race conditions or UI inconsistencies when closing multiple tabs.

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)

439-469: Empty catch blocks silently swallow errors.

Both handleCloseTab and handleRevertChanges have empty catch blocks. Consider logging errors for debugging, even if no user-facing action is required.

🔎 Suggested improvement:

     } catch (err) { }
+    } catch (err) {
+      console.error('Failed to close tab:', err);
+    }

-    } catch (err) { }
+    } catch (err) {
+      console.error('Failed to revert changes:', err);
+    }

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.

packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js (4)

1-1: Remove unused import.

The styled import is not used anywhere in this file. Based on learnings, the project typically uses styled components as wrappers, but this file now only exports a global style.

🔎 Apply this diff:

-import styled, { createGlobalStyle } from 'styled-components';
+import { createGlobalStyle } from 'styled-components';

---

3-173: Consider the implications of using global styles for component-specific styling.

Using createGlobalStyle to target third-party library classes (.tippy-box) globally may cause unintended side effects if other Tippy instances exist elsewhere in the app. This approach deviates from the project's pattern of using styled component wrappers for component-specific styles.

While this may be intentional for portal-rendered dropdowns, consider:

• Adding a more specific class selector to scope these styles only to MenuDropdown instances
• Or documenting that these styles apply to all Tippy instances app-wide

Based on learnings, styled components are typically used as wrappers to define both self and children component styles in this codebase.

---

13-13: Avoid !important if possible.

The !important flag is generally a code smell, though it may be necessary here to override Tippy's default inline styles. If this is required to override library defaults, consider adding a comment explaining why.

---

116-122: Extract inline color calculation to a helper function.

The inline RGBA conversion logic reduces readability and is repeated in similar patterns elsewhere in the codebase. Consider extracting this to a utility function.

🔎 Suggested approach:

Create a utility function in a theme/color helpers file:

// utils/colors.js
export const hexToRgba = (hex, alpha) => {
  const cleanHex = hex.replace('#', '');
  const r = parseInt(cleanHex.substring(0, 2), 16);
  const g = parseInt(cleanHex.substring(2, 4), 16);
  const b = parseInt(cleanHex.substring(4, 6), 16);
  return `rgba(${r}, ${g}, ${b}, ${alpha})`;
};

Then use it:

           &:hover {
-            background-color: ${({ theme }) => {
-              const hex = theme.colors.text.danger.replace('#', '');
-              const r = parseInt(hex.substring(0, 2), 16);
-              const g = parseInt(hex.substring(2, 4), 16);
-              const b = parseInt(hex.substring(4, 6), 16);
-              return `rgba(${r}, ${g}, ${b}, 0.04)`; // 4% opacity
-            }} !important;
+            background-color: ${({ theme }) => hexToRgba(theme.colors.text.danger, 0.04)} !important;

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (2)

479-503: Good fix for async/await pattern!

The handlers now correctly use Promise.all with await, addressing the previous review comment about forEach with async functions. However, handleCloseSavedTabs (line 494) is inconsistent—it doesn't follow the async pattern and doesn't await tab closures like the others.

Consider making handleCloseSavedTabs consistent:

🔎 Apply this diff:

-  function handleCloseSavedTabs() {
+  async function handleCloseSavedTabs() {
     const items = flattenItems(collection?.items);
     const savedTabs = items?.filter?.((item) => !hasRequestChanges(item));
     const savedTabIds = savedTabs?.map((item) => item.uid) || [];
-    dispatch(closeTabs({ tabUids: savedTabIds }));
+    await Promise.all(savedTabIds.map((uid) => handleCloseTab(uid)));
   }

---

505-555: Optimize useMemo dependency array.

The dispatch function from useDispatch() is stable and doesn't need to be in the dependency array. Including it causes unnecessary recalculations.

🔎 Apply this diff:

-  ], [currentTabUid, currentTabItem, hasOtherTabs, hasLeftTabs, hasRightTabs, collection, collectionRequestTabs, tabIndex, dispatch]);
+  ], [currentTabUid, currentTabItem, hasOtherTabs, hasLeftTabs, hasRightTabs, collection, collectionRequestTabs, tabIndex]);

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component

Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors