feat: Increase visibility of text in Request tabs#6243
feat: Increase visibility of text in Request tabs#6243bijin-bruno merged 11 commits intousebruno:mainfrom
Conversation
…and adjust styles
WalkthroughReplaces per-tab close/draft icons with a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)
214-227: Inconsistent padding in RequestTabNotFound path.The item-not-found path uses
px-1while other tab containers in this file usepx-3(lines 111, 234). This creates visual inconsistency across tab states.Apply this diff to align padding:
- <StyledWrapper - className="flex items-center justify-between tab-container px-1" + <StyledWrapper + className="flex items-center justify-between tab-container px-3"
🧹 Nitpick comments (1)
packages/bruno-app/src/components/RequestTabs/ExampleTab/index.js (1)
104-122: Consider removing left padding for consistency.The tab-label uses
pl-2while RequestTab (line 271) removed left padding from its tab-label. This creates a subtle visual inconsistency between ExampleTab and RequestTab.Apply this diff if alignment with RequestTab is desired:
<div - className={`flex items-center tab-label pl-2 ${tab.preview ? 'italic' : ''}`} + className={`flex items-center tab-label ${tab.preview ? 'italic' : ''}`}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/bruno-app/src/components/RequestTabs/ExampleTab/index.js(5 hunks)packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js(1 hunks)packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js(1 hunks)packages/bruno-app/src/components/RequestTabs/RequestTab/SpecialTab.js(1 hunks)packages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.js(1 hunks)packages/bruno-app/src/components/RequestTabs/RequestTab/index.js(5 hunks)packages/bruno-app/src/components/RequestTabs/StyledWrapper.js(3 hunks)packages/bruno-app/src/components/RequestTabs/index.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-app/src/components/RequestTabs/index.jspackages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.jspackages/bruno-app/src/components/RequestTabs/RequestTab/SpecialTab.jspackages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.jspackages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.jspackages/bruno-app/src/components/RequestTabs/StyledWrapper.jspackages/bruno-app/src/components/RequestTabs/RequestTab/index.jspackages/bruno-app/src/components/RequestTabs/ExampleTab/index.js
🧠 Learnings (3)
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components should be used as wrappers to define both self and children component styles; Tailwind classes should be used specifically for layout-based styles
Applied to files:
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.jspackages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.jspackages/bruno-app/src/components/RequestTabs/StyledWrapper.js
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS may change layout, but Tailwind classes should not define colors
Applied to files:
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.jspackages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.jspackages/bruno-app/src/components/RequestTabs/StyledWrapper.js
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{jsx,tsx} : In styled components and React components using styled components, use the theme prop to manage CSS colors instead of CSS variables
Applied to files:
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js
🧬 Code graph analysis (4)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js (5)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js (1)
StyledWrapper(3-99)packages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.js (1)
StyledWrapper(3-43)packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)
hasChanges(210-210)packages/bruno-app/src/components/RequestTabs/RequestTab/DraftTabIcon.js (1)
DraftTabIcon(1-13)packages/bruno-app/src/components/RequestTabs/RequestTab/CloseTabIcon.js (1)
CloseTabIcon(1-9)
packages/bruno-app/src/components/RequestTabs/RequestTab/SpecialTab.js (2)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js (1)
GradientCloseButton(6-19)packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (2)
hasDraft(105-105)handleCloseClick(38-46)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js (1)
packages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.js (1)
StyledWrapper(3-43)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (4)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js (1)
StyledWrapper(3-99)packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js (1)
GradientCloseButton(6-19)packages/bruno-app/src/components/RequestPane/WsQueryUrl/index.js (1)
hasChanges(44-44)packages/bruno-app/src/components/RequestPane/QueryUrl/index.js (1)
hasChanges(29-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
🔇 Additional comments (12)
packages/bruno-app/src/components/RequestTabs/index.js (1)
90-90: Padding reduction aligns with goal of improving tab text visibilityChanging from larger to smaller left padding on the tab container gives more horizontal room for tab labels without affecting behavior; this looks good and consistent with the rest of the layout.
packages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.js (1)
4-42: LGTM! Solid implementation of the text fade effect.The positioning, flexbox, and mask-image gradient work together cleanly to reveal text progressively on hover. The styling aligns with the PR objective of increasing text visibility in request tabs.
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js (1)
6-19: LGTM! Clean component consolidation.The GradientCloseButton successfully consolidates the close/draft icon logic into a single reusable component. The conditional className approach and dual-icon structure work well with the CSS-driven visibility in StyledWrapper.
packages/bruno-app/src/components/RequestTabs/RequestTab/SpecialTab.js (1)
6-57: LGTM! Consistent tab rendering structure.The refactoring standardizes all special tab types with uniform icon sizing (14px) and a consistent
tab-nameclass. The GradientCloseButton integration simplifies the close control logic.packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js (1)
3-99: LGTM! Well-structured gradient overlay with state-driven visibility.The styled wrapper effectively creates a gradient fade overlay that reveals on hover or when changes are present. The icon-switching logic (draft vs. close) is cleanly implemented through display toggles.
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (2)
107-107: LGTM! Consistent draft state handling.The
hasFolderDraftcomputation follows the same pattern ashasDraftand correctly identifies draft state for folder settings tabs.
300-312: LGTM! Clean migration to GradientCloseButton.The close button logic correctly preserves the hasChanges-aware behavior, triggering confirmation when needed and performing direct closes otherwise.
packages/bruno-app/src/components/RequestTabs/ExampleTab/index.js (2)
61-77: LGTM! Consistent padding across tab states.Both the item-not-found and main tab paths now use
px-3, ensuring visual consistency.
123-134: LGTM! Clean GradientCloseButton integration.The close button correctly implements hasChanges-aware behavior, matching the pattern used in RequestTab.
packages/bruno-app/src/components/RequestTabs/StyledWrapper.js (3)
6-15: LGTM! Clean pseudo-element border implementation.The ::after pseudo-element approach for the bottom border is a standard technique that provides better z-index control than a direct border on the wrapper.
64-101: LGTM! Elegant curved tab notches.The ::before and ::after pseudo-elements with box-shadow create smooth, browser-native tab curves that integrate seamlessly with the bottom border. The z-index layering ensures proper overlap.
31-46: LGTM! Improved tab spacing and sizing.The increased max-width (150px → 180px) and added min-width (80px) provide better text visibility while maintaining reasonable tab sizes. The margin-bottom and padding adjustments align with the new curved tab design.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js (1)
1-21: Solid extraction; consider minor polish for className, docs, and accessibilityThe component behavior looks correct and aligns with how the tests are using
data-testid="request-tab-close-icon". To tighten things up:
- Avoid trailing space in
classNameby only prefixing' has-changes'when needed.- Add a short JSDoc on the component to describe
hasChangessemantics (drives draft/unsaved-visual state).- Since this is effectively a button, consider adding
role="button"andaria-label(and possiblytabIndex={0}/keyboard handling) so the close control is accessible beyond mouse clicks.Example diff:
-const GradientCloseButton = ({ onClick, hasChanges = false }) => { - return ( - <StyledWrapper className={`close-gradient ${hasChanges ? 'has-changes' : ''}`}> - <div className="close-icon-container" onClick={onClick} data-testid="request-tab-close-icon"> +/** + * Close control for request tabs, optionally showing draft/unsaved state via `hasChanges`. + */ +const GradientCloseButton = ({ onClick, hasChanges = false }) => { + return ( + <StyledWrapper className={`close-gradient${hasChanges ? ' has-changes' : ''}`}> + <div + className="close-icon-container" + role="button" + aria-label="Close request tab" + onClick={onClick} + data-testid="request-tab-close-icon" + >tests/grpc/method-search/grpc-method-search.spec.ts (1)
18-20: Align close-tab interaction with hover pattern to reduce flakinessHere you click the close icon without hovering the tab first:
await page.getByRole('tab', { name: 'gRPC sayHello' }).getByTestId('request-tab-close-icon').click();Elsewhere in this PR, the close icon is revealed on tab hover before clicking. To keep behavior consistent and avoid flakiness if the icon is hover-only, consider:
- await page.getByRole('tab', { name: 'gRPC sayHello' }).getByTestId('request-tab-close-icon').click(); + const requestTab = page.getByRole('tab', { name: 'gRPC sayHello' }); + await requestTab.hover(); + await requestTab.getByTestId('request-tab-close-icon').click();tests/preferences/autosave/autosave.spec.ts (1)
57-63: Good synchronization with draft state; consider DRYing close-tab behaviorThe updated autosave tests look much more robust now:
- Waiting for
.has-changes-iconto become visible and then disappear withnot.toBeVisible({ timeout: 5000 })is a solid, state-based way to assert autosave.- Moving the mouse away before checking the draft icon (because hover swaps it for the close icon) matches the new UI behavior.
- Hovering the
.request-tabbefore clickingrequest-tab-close-iconis consistent with the new GradientCloseButton interaction.Given how often this pattern appears:
const requestTab = page.locator('.request-tab').filter({ has: page.locator('.tab-label', { hasText: '...' }) }); await requestTab.hover(); await requestTab.getByTestId('request-tab-close-icon').click();it may be worth extracting a small utility (e.g.
closeRequestTab(page, 'Test Request')) intests/utils/pageand reusing it across specs to keep tests concise and consistent. Based on learnings, this keeps new behavior well-covered while staying maintainable.Also applies to: 68-70, 106-108, 155-157, 191-193
tests/protobuf/manage-protofile.spec.ts (1)
38-44: Improved stability via explicit waits; close-tab pattern could be sharedThe extra waits you added here are helpful:
- Asserting visibility of specific table cells like
'product.proto', its path, import paths, and invalid entries ensures the protobuf tables are fully loaded before assertions or mutations.- Similarly, waiting for collection-path cells before proceeding should reduce race conditions.
The new cleanup steps:
const requestTab = page.getByRole('tab', { name: 'gRPC sayHello' }); await requestTab.hover(); await requestTab.getByTestId('request-tab-close-icon').click(); await page.getByRole('button', { name: "Don't Save" }).click();correctly mirror the updated tab-close UI and should be reliable.
Given this exact pattern appears in multiple tests in this file (and others), consider a small shared helper (e.g.
closeGrpcSayHelloTabWithoutSaving(page)) to centralize the sequence and keep specs focused on behavior rather than mechanics.Also applies to: 49-52, 53-56, 70-72, 109-112, 137-140, 181-184
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.js(1 hunks)tests/grpc/method-search/grpc-method-search.spec.ts(1 hunks)tests/preferences/autosave/autosave.spec.ts(4 hunks)tests/protobuf/manage-protofile.spec.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
tests/protobuf/manage-protofile.spec.tstests/preferences/autosave/autosave.spec.tspackages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/index.jstests/grpc/method-search/grpc-method-search.spec.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Files:
tests/protobuf/manage-protofile.spec.tstests/preferences/autosave/autosave.spec.tstests/grpc/method-search/grpc-method-search.spec.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/protobuf/manage-protofile.spec.tstests/preferences/autosave/autosave.spec.tstests/grpc/method-search/grpc-method-search.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{test,spec}.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes; update corresponding tests when code is added, removed, or significantly modified
Applied to files:
tests/protobuf/manage-protofile.spec.tstests/preferences/autosave/autosave.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Windows
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: Playwright E2E Tests
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js(1 hunks)packages/bruno-app/src/components/RequestTabs/RequestTab/SpecialTab.js(2 hunks)packages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.js(1 hunks)packages/bruno-app/src/components/RequestTabs/RequestTab/index.js(7 hunks)packages/bruno-app/src/components/RequestTabs/StyledWrapper.js(3 hunks)packages/bruno-app/src/components/RequestTabs/index.js(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/bruno-app/src/components/RequestTabs/RequestTab/SpecialTab.js
- packages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-app/src/components/RequestTabs/StyledWrapper.jspackages/bruno-app/src/components/RequestTabs/RequestTab/index.jspackages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.jspackages/bruno-app/src/components/RequestTabs/index.js
🧠 Learnings (4)
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components should be used as wrappers to define both self and children component styles; Tailwind classes should be used specifically for layout-based styles
Applied to files:
packages/bruno-app/src/components/RequestTabs/StyledWrapper.jspackages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS may change layout, but Tailwind classes should not define colors
Applied to files:
packages/bruno-app/src/components/RequestTabs/StyledWrapper.jspackages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js
📚 Learning: 2025-12-02T20:48:05.634Z
Learnt from: NikHillAgar
Repo: usebruno/bruno PR: 6279
File: packages/bruno-app/src/components/RequestPane/PromptVariables/PromptVariablesModal/index.js:79-86
Timestamp: 2025-12-02T20:48:05.634Z
Learning: In React 19+, ref callbacks support returning cleanup functions. When a ref callback returns a function, React will call it when the element unmounts or the ref changes, similar to useEffect cleanup. This is documented at https://react.dev/learn/manipulating-the-dom-with-refs#how-to-manage-a-list-of-refs-using-a-ref-callback
Applied to files:
packages/bruno-app/src/components/RequestTabs/RequestTab/index.jspackages/bruno-app/src/components/RequestTabs/index.js
📚 Learning: 2025-12-03T08:09:57.124Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-03T08:09:57.124Z
Learning: Applies to **/*.{jsx,tsx} : In styled components and React components using styled components, use the theme prop to manage CSS colors instead of CSS variables
Applied to files:
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js (1)
packages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.js (1)
StyledWrapper(3-30)
🪛 Biome (2.1.2)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
[error] 232-232: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 233-233: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 235-235: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
packages/bruno-app/src/components/RequestTabs/index.js
[error] 75-75: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
🔇 Additional comments (8)
packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js (1)
1-99: LGTM! Theme integration follows coding standards.The gradient close button styling is well-implemented with proper theme prop usage for colors and smooth transitions. The conditional visibility logic for draft/close icons works correctly.
Based on learnings, the use of theme props for color management aligns with the project's styled-components pattern.
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (2)
111-111: Padding adjustments align with gradient button layout.The increased horizontal padding (px-1 → px-3) and removal of left padding from tab-label accommodate the new GradientCloseButton and improve overall spacing.
Also applies to: 263-263, 300-300
329-341: GradientCloseButton integration looks correct.The new component properly receives the hasChanges state and preserves the existing close behavior including WebSocket cleanup and confirmation modals for unsaved changes.
packages/bruno-app/src/components/RequestTabs/index.js (2)
28-40: Well-implemented memoization pattern.The createSetHasOverflow callback correctly uses useCallback and includes an optimization to prevent unnecessary state updates when the overflow state hasn't changed.
136-169: Clean scroll container restructuring.The introduction of a dedicated scroll container with proper ref management and the threading of overflow-related props (hasOverflow, setHasOverflow) to child components maintains existing functionality while enabling the new overflow detection feature.
packages/bruno-app/src/components/RequestTabs/StyledWrapper.js (3)
4-33: Clean implementation of scroll container and border styling.The use of a ::after pseudo-element for the bottom border and the dedicated scroll container with hidden scrollbar improves visual consistency and UX.
72-111: Overflow mask implementation handles text truncation effectively.The gradient masks applied via mask-image on .has-overflow tabs provide a smooth fade effect for overflowing text, with different masks for hover and non-hover states. This is a polished UX touch.
113-150: Active tab decorations add visual polish.The ::before and ::after pseudo-elements create rounded corner notches that seamlessly connect the active tab to the content area. The implementation using box-shadow and border-radius is elegant.
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)
261-286: Critical: Remove duplicate useEffect that violates Rules of Hooks.This useEffect is declared after early returns (lines 152-257), breaking React's Rules of Hooks. It's also a duplicate of the effect at lines 54-80.
React requires all hooks to be called in the same order on every render. This placement will cause runtime errors when switching between tab types.
Apply this diff to remove the duplicate:
const isWS = item.type === 'ws-request'; - - useEffect(() => { - const checkOverflow = () => { - if (tabNameRef.current && setHasOverflow) { - const hasOverflow = tabNameRef.current.scrollWidth > tabNameRef.current.clientWidth; - if (lastOverflowStateRef.current !== hasOverflow) { - lastOverflowStateRef.current = hasOverflow; - setHasOverflow(hasOverflow); - } - } - }; - - const timeoutId = setTimeout(checkOverflow, 0); - - const resizeObserver = new ResizeObserver(() => { - checkOverflow(); - }); - - if (tabNameRef.current) { - resizeObserver.observe(tabNameRef.current); - } - - return () => { - clearTimeout(timeoutId); - resizeObserver.disconnect(); - }; - }, [item.name, method, setHasOverflow]); return (packages/bruno-app/src/components/RequestTabs/index.js (1)
94-109: Critical: Remove duplicate useEffect that violates Rules of Hooks.This useEffect is declared after early returns (lines 83-89), breaking React's Rules of Hooks. It's a duplicate of the effect at lines 46-63, which already handles overflow detection correctly.
All hooks must be called in the same order on every render. This will cause errors if
activeTabUidoractiveTabbecome undefined after being defined.Apply this diff to remove the duplicate:
const maxTablistWidth = screenWidth - effectiveSidebarWidth - 150; - - useEffect(() => { - const checkOverflow = () => { - if (tabsRef.current && scrollContainerRef.current) { - const hasOverflow = tabsRef.current.scrollWidth > scrollContainerRef.current.clientWidth; - setShowChevrons(hasOverflow); - } - }; - - checkOverflow(); - const resizeObserver = new ResizeObserver(checkOverflow); - if (scrollContainerRef.current) { - resizeObserver.observe(scrollContainerRef.current); - } - - return () => resizeObserver.disconnect(); - }, [collectionRequestTabs.length, screenWidth, leftSidebarWidth, sidebarCollapsed]); const leftSlide = () => {
🧹 Nitpick comments (1)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)
401-401: Consider logging errors instead of silent catch blocks.Empty catch blocks at lines 401 and 420 make debugging difficult. At minimum, log the error to the console.
Apply this diff:
- } catch (err) { } + } catch (err) { + console.error('Error closing tab:', err); + }And:
- } catch (err) { } + } catch (err) { + console.error('Error reverting changes:', err); + }Also applies to: 420-420
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js(10 hunks)packages/bruno-app/src/components/RequestTabs/index.js(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation, never tabs
Use single quotes for strings instead of double quotes
Always add semicolons at the end of statements
No trailing commas in code
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line with a minimum of 2 elements for multiline formatting
No newlines inside function parentheses
Space before and after the arrow in arrow functions:() => {}
No space between function name and parentheses:func()notfunc ()
Semicolons should go at the end of the line, not on a new line
Function names should be concise and descriptive
Add JSDoc comments to provide details to abstractions
Avoid single-line abstractions where all that is being done is increasing the call stack with one additional function
Add meaningful comments to explain complex code flow instead of obvious comments
Files:
packages/bruno-app/src/components/RequestTabs/RequestTab/index.jspackages/bruno-app/src/components/RequestTabs/index.js
🧠 Learnings (1)
📚 Learning: 2025-12-02T20:48:05.634Z
Learnt from: NikHillAgar
Repo: usebruno/bruno PR: 6279
File: packages/bruno-app/src/components/RequestPane/PromptVariables/PromptVariablesModal/index.js:79-86
Timestamp: 2025-12-02T20:48:05.634Z
Learning: In React 19+, ref callbacks support returning cleanup functions. When a ref callback returns a function, React will call it when the element unmounts or the ref changes, similar to useEffect cleanup. This is documented at https://react.dev/learn/manipulating-the-dom-with-refs#how-to-manage-a-list-of-refs-using-a-ref-callback
Applied to files:
packages/bruno-app/src/components/RequestTabs/RequestTab/index.jspackages/bruno-app/src/components/RequestTabs/index.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/RequestTabs/index.js (2)
packages/bruno-app/src/components/RequestTabs/DraggableTab.js (1)
DraggableTab(4-45)packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (2)
dispatch(26-26)RequestTab(25-370)
🪛 Biome (2.1.2)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
[error] 261-261: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
packages/bruno-app/src/components/RequestTabs/index.js
[error] 94-94: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (14)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (6)
1-1: Imports look good.The new imports (
useMemo,useEffect) are properly added to support the overflow detection logic.
29-30: Well-placed refs for overflow detection.The refs are correctly declared at the top of the component, ensuring they're available unconditionally on every render.
40-52: Method computation looks correct.The memoized method resolution properly handles different request types. The
[item]dependency should cover changes to item properties assuming Redux immutability.
54-80: Overflow detection logic is sound.The ResizeObserver approach with optimized state updates (via
lastOverflowStateRef) is a good pattern. Proper cleanup ensures no memory leaks.
355-367: GradientCloseButton integration looks good.The close button properly handles both saved and unsaved tab states, with appropriate confirmation prompts for tabs with changes.
155-155: Layout and styling updates support the visibility improvements.The increased padding (
px-3) and added ref for overflow detection align well with the PR objective.Also applies to: 289-289, 326-326, 342-342
packages/bruno-app/src/components/RequestTabs/index.js (8)
1-1: New state management for overflow detection looks good.The addition of
scrollContainerRef,tabOverflowStates, andshowChevronsprovides the foundation for the overflow tracking feature.Also applies to: 17-17, 19-20
28-40: Well-designed overflow state factory.The
createSetHasOverflowcallback with optimization checks prevents unnecessary re-renders. The empty dependency array is correct since the factory doesn't close over any changing values.
42-44: Proper variable hoisting to support unconditional hooks.Moving these declarations before early returns allows them to be used in hook dependencies while maintaining the Rules of Hooks.
46-63: Overflow detection correctly positioned and implemented.The ResizeObserver approach with comprehensive dependencies ensures chevrons appear/disappear appropriately. The immediate
checkOverflow()call plus observer might trigger twice initially, but this is harmless.
112-123: Scroll handlers properly updated for new container.The handlers correctly target
scrollContainerRefwith smooth scrolling behavior and safe optional chaining.
155-188: Scroll container structure properly implements overflow detection.The nested structure (scroll container → tabs list → individual tabs) correctly enables both global overflow detection (for chevrons) and per-tab overflow tracking.
180-181: Per-tab overflow props correctly wired.The
hasOverflowandsetHasOverflowprops complete the overflow tracking loop, enabling both parent-level chevron display and per-tab styling.
139-139: Padding adjustment aligns with new layout.The reduced left padding accommodates the scrollable tab container structure.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)
261-286: Critical: Remove duplicate useEffect that violates Rules of Hooks.Lines 261-286 contain a duplicate
useEffectdeclared after early returns (lines 179, 193), violating React's Rules of Hooks. This will cause runtime errors when switching between tab types, as hooks must be called in the same order on every render.The overflow detection logic already exists at lines 54-80 (correctly placed at the top level). This duplicate block must be removed entirely.
Apply this diff:
const isWS = item.type === 'ws-request'; - - useEffect(() => { - const checkOverflow = () => { - if (tabNameRef.current && setHasOverflow) { - const hasOverflow = tabNameRef.current.scrollWidth > tabNameRef.current.clientWidth; - if (lastOverflowStateRef.current !== hasOverflow) { - lastOverflowStateRef.current = hasOverflow; - setHasOverflow(hasOverflow); - } - } - }; - - const timeoutId = setTimeout(checkOverflow, 0); - - const resizeObserver = new ResizeObserver(() => { - checkOverflow(); - }); - - if (tabNameRef.current) { - resizeObserver.observe(tabNameRef.current); - } - - return () => { - clearTimeout(timeoutId); - resizeObserver.disconnect(); - }; - }, [item.name, method, setHasOverflow]); return (
🧹 Nitpick comments (1)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)
54-80: Simplify dependency array.The dependency array includes both
itemanditem?.name. Sinceitem?.namechanges always implyitemhas changed (by reference equality), including both is redundant. Consider simplifying to[item, method, setHasOverflow].- }, [item, item?.name, method, setHasOverflow]); + }, [item, method, setHasOverflow]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.js(1 hunks)packages/bruno-app/src/components/RequestTabs/RequestTab/index.js(10 hunks)packages/bruno-app/src/components/RequestTabs/StyledWrapper.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/RequestTabs/RequestTab/index.jspackages/bruno-app/src/components/RequestTabs/StyledWrapper.js
🧠 Learnings (3)
📚 Learning: 2025-12-02T20:48:05.634Z
Learnt from: NikHillAgar
Repo: usebruno/bruno PR: 6279
File: packages/bruno-app/src/components/RequestPane/PromptVariables/PromptVariablesModal/index.js:79-86
Timestamp: 2025-12-02T20:48:05.634Z
Learning: In React 19+, ref callbacks support returning cleanup functions. When a ref callback returns a function, React will call it when the element unmounts or the ref changes, similar to useEffect cleanup. This is documented at https://react.dev/learn/manipulating-the-dom-with-refs#how-to-manage-a-list-of-refs-using-a-ref-callback
Applied to files:
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
📚 Learning: 2025-12-05T20:31:32.975Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:32.975Z
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
Applied to files:
packages/bruno-app/src/components/RequestTabs/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:32.975Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:32.975Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors
Applied to files:
packages/bruno-app/src/components/RequestTabs/StyledWrapper.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (2)
packages/bruno-app/src/components/RequestTabs/RequestTab/StyledWrapper.js (1)
StyledWrapper(3-29)packages/bruno-app/src/components/RequestTabs/RequestTab/GradientCloseButton/StyledWrapper.js (1)
StyledWrapper(3-99)
🪛 Biome (2.1.2)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
[error] 261-261: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Windows
🔇 Additional comments (9)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (4)
1-1: LGTM: Imports and initial hooks properly positioned.The additions of
useEffect,GradientCloseButton, and new props (hasOverflow,setHasOverflow) are correctly placed. The refs (tabNameRef,lastOverflowStateRef) are declared at the top level before any early returns, adhering to React's Rules of Hooks.Also applies to: 20-20, 25-25, 29-30
40-52: LGTM: Method memoization is appropriate.The
useMemohook efficiently computes the method string for different request types (gRPC, WS, GraphQL) and correctly memoizes based on theitemdependency.
151-156: LGTM: Consistent styling for special tabs.The padding increase (
px-1→px-2) andonMouseUphandler addition align with the visibility improvements across all tab types.
289-289: LGTM: Tab rendering improvements align with PR goals.The padding increase, ref attachment for overflow detection, and
GradientCloseButtonreplacement enhance tab visibility and simplify close button logic. The removal of left padding on.tab-nameis consistent with the new layout approach.Also applies to: 326-326, 342-342, 355-367
packages/bruno-app/src/components/RequestTabs/StyledWrapper.js (5)
4-33: LGTM: Scroll container implementation is sound.The
::afterpseudo-element for the bottom border provides better z-index layering with active tab decorations. The scroll container properly hides scrollbars cross-browser using both WebKit-specific and standard properties, with the padding/margin trick to clip the scrollbar area.
35-77: LGTM: Base tab styles support improved visibility.The increased
max-width(150px → 180px), newmin-width(80px), and adjusted spacing provide more room for tab content. The:not(.active)selector inversion is cleaner than the previous approach.
79-111: LGTM: Overflow masking provides polished UX.The gradient masking on
.has-overflowtabs creates a smooth fade-out effect for overflowing text. The different masks for hover vs non-hover states (more text visible on hover) enhance usability. Cross-browser compatibility is properly handled with-webkit-mask-image.
113-150: LGTM: Active tab decorations are well-crafted.The
::beforeand::afterpseudo-elements create polished curved notches connecting the active tab to the bottom border. The box-shadow technique to fill the notch curves is clever. Z-index layering and negative margins ensure proper visual hierarchy.
152-202: LGTM: Short tab and chevron styles are consistent.The fixed dimensions for short tabs (32px width, 20px icons) and the
has-chevronspadding adjustment maintain consistency across different tab states and scroll scenarios. Styling properly uses theme tokens throughout.Based on learnings, Styled Components are appropriately used for colors and theming while layout uses Tailwind classes in the component files.
|
|
||
| dispatch(closeTabs({ tabUids: [tabUid] })); | ||
| } catch (err) {} | ||
| } catch (err) { } |
There was a problem hiding this comment.
Add minimal error logging in catch blocks.
Empty catch blocks at lines 401 and 420 silently swallow errors, making debugging difficult. Consider adding at least a console warning to track failures in tab operations.
- } catch (err) { }
+ } catch (err) {
+ console.warn('Failed to close tab:', err);
+ }- } catch (err) { }
+ } catch (err) {
+ console.warn('Failed to revert changes:', err);
+ }Also applies to: 420-420
🤖 Prompt for AI Agents
In packages/bruno-app/src/components/RequestTabs/RequestTab/index.js around
lines 401 and 420, the catch blocks are empty and silently swallow errors;
update each catch to log a minimal warning (e.g., console.warn or logger.warn)
including the caught error and a short contextual message so failures in tab
operations can be traced; ensure you preserve existing control flow after
logging and do not rethrow unless intentional.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-app/src/components/RequestTabs/index.js (1)
28-40: Consider memoizing per-tab callbacks to reduce allocations (optional).
createSetHasOverflow(tab.uid)creates a new function for each tab on every render. SinceRequestTabisn't memoized, this doesn't cause re-render issues, but it does allocate new functions each time.If you want to optimize, memoize the callbacks in a Map:
- const createSetHasOverflow = useCallback((tabUid) => { - return (hasOverflow) => { + const setHasOverflowCallbacks = useRef(new Map()); + + const createSetHasOverflow = useCallback((tabUid) => { + if (!setHasOverflowCallbacks.current.has(tabUid)) { + setHasOverflowCallbacks.current.set(tabUid, (hasOverflow) => { - setTabOverflowStates((prev) => { - if (prev[tabUid] === hasOverflow) { - return prev; - } - return { - ...prev, - [tabUid]: hasOverflow - }; - }); - }; + setTabOverflowStates((prev) => { + if (prev[tabUid] === hasOverflow) return prev; + return { ...prev, [tabUid]: hasOverflow }; + }); + }); + } + return setHasOverflowCallbacks.current.get(tabUid); }, []);However, for typical tab counts, the current approach is perfectly acceptable.
Also applies to: 164-164
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-app/src/components/RequestTabs/index.js(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/components/RequestTabs/index.js
🧠 Learnings (1)
📚 Learning: 2025-12-02T20:48:05.634Z
Learnt from: NikHillAgar
Repo: usebruno/bruno PR: 6279
File: packages/bruno-app/src/components/RequestPane/PromptVariables/PromptVariablesModal/index.js:79-86
Timestamp: 2025-12-02T20:48:05.634Z
Learning: In React 19+, ref callbacks support returning cleanup functions. When a ref callback returns a function, React will call it when the element unmounts or the ref changes, similar to useEffect cleanup. This is documented at https://react.dev/learn/manipulating-the-dom-with-refs#how-to-manage-a-list-of-refs-using-a-ref-callback
Applied to files:
packages/bruno-app/src/components/RequestTabs/index.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/RequestTabs/index.js (2)
packages/bruno-app/src/components/RequestTabs/DraggableTab.js (1)
DraggableTab(4-45)packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (2)
dispatch(26-26)RequestTab(25-370)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Windows
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
- GitHub Check: Unit Tests
🔇 Additional comments (2)
packages/bruno-app/src/components/RequestTabs/index.js (2)
1-63: Hook ordering corrected—overflow detection implemented correctly.The critical issue from the previous review (useEffect after early returns) has been properly resolved. All hooks now appear before any conditional returns, and the overflow detection logic is sound:
createSetHasOverflowcorrectly memoizes the setter factory- Computed values (
activeTab,activeCollection,collectionRequestTabs) are appropriately placed after hooks but before early returns, as required by the dependencies- ResizeObserver setup and cleanup are correct
122-171: Layout restructuring looks solid.The new scroll-container structure correctly separates concerns:
- Outer
divwithscrollContainerRefhandles scrolling behavior- Inner
ulwithtabsRefmeasures content width for overflow detection- Per-tab overflow state flows cleanly via
hasOverflowandsetHasOverflowpropsThe padding adjustment (
pl-4→pl-2) and the integration withDraggableTabmaintain consistency with the overall tab visibility improvements.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/preferences/autosave/autosave.spec.ts (3)
72-75: Close‑tab interaction via hover + test id looks solid; consider locator variableUsing
requestTab.hover()andrequestTab.getByTestId('request-tab-close-icon').click()matches the new UI behavior and makes the tests more resilient than CSS‑only selectors. If you want to tighten things further, you could assign the close control to a locator variable so you can both assert visibility and click it:- await requestTab.hover(); - await requestTab.getByTestId('request-tab-close-icon').click(); + await requestTab.hover(); + const closeButton = requestTab.getByTestId('request-tab-close-icon'); + await expect(closeButton).toBeVisible(); + await closeButton.click();As per coding guidelines, using locator variables improves readability and reuse.
Also applies to: 189-191
111-113: Mouse move to (0, 0) works but is somewhat layout‑sensitiveThe
page.mouse.move(0, 0)trick is a reasonable way to clear the tab hover so the draft icon becomes visible, but it does couple the test to a specific coordinate. If the layout changes and something interactive ends up at (0, 0), this could behave differently. A slightly more robust option is to move/hover over a known neutral element (e.g., a top‑level container or the sidebar) via a locator:await page.locator('.sidebar').hover(); // or another stable non-tab areaNot a blocker, but worth considering for long‑term stability.
Also applies to: 156-158
181-185: Avoid fixedwaitForTimeoutbefore autosave assertionHere you wait 1s and then also use
not.toBeVisible({ timeout: 10000 })on the draft icon:await page.waitForTimeout(1000); const requestTab = ... await expect(requestTab.locator('.has-changes-icon')).not.toBeVisible({ timeout: 10000 });The explicit timeout on
not.toBeVisiblealready handles waiting for autosave; the extrawaitForTimeout(1000)just slows the test and can still be flaky. You can simplify to:- await page.waitForTimeout(1000); - const requestTab = page.locator('.request-tab').filter({ has: page.locator('.tab-label', { hasText: 'Draft Request' }) }); await expect(requestTab.locator('.has-changes-icon')).not.toBeVisible({ timeout: 10000 });If this was added to fight flakiness, it’s better to adjust the expectation (or target a more precise ready state) than rely on a fixed sleep. As per coding guidelines, we want to minimize
page.waitForTimeout().
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/preferences/autosave/autosave.spec.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
tests/preferences/autosave/autosave.spec.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/preferences/autosave/autosave.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Playwright E2E Tests
- GitHub Check: SSL Tests - Windows
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
- GitHub Check: SSL Tests - macOS
- GitHub Check: SSL Tests - Linux
🔇 Additional comments (1)
tests/preferences/autosave/autosave.spec.ts (1)
62-67: Good shift to state-based waits for autosaveWaiting for the draft icon to first appear and then disappear is a solid, state-driven pattern and should be much less flaky than fixed timeouts. No changes needed here.
| align-items: center; | ||
| width: 24px; | ||
| height: 24px; | ||
| border-radius: 4px; |
There was a problem hiding this comment.
Border radius should be consistent with other containers and should be coming from themes
Demo:
Screen.Recording.2025-11-28.at.6.40.35.PM.mov
Summary by CodeRabbit
New Features
UI Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.