feature: Implement draggable tabs and request tab order persistence #6976
Conversation
kanakkholwal
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
WalkthroughImplements draggable request-tab reordering with persistence (multi-scope: global/collection/folder/request), adds pane visibility toggles for request/response, persists requestTabOrder across formats, updates ResponsiveTabs with DraggableTab, and wires Redux actions/reducers to manage ordering and pane visibility. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Request Pane UI
participant Drag as DraggableTab
participant RDX as Redux
participant FS as FileStore
User->>UI: Drag tab (index A -> B)
UI->>Drag: moveTab(A, B)
Drag->>UI: Call onTabReorder(newOrder)
UI->>RDX: Dispatch updateRequestTabOrder(collectionUid, itemUid, newOrder)
RDX->>RDX: Read preferences.requestTabOrderPersistenceScope
alt Scope = global
RDX->>RDX: Update preferences.requestTabOrder[type]
else Scope = request
RDX->>FS: Save request with requestTabOrder
else Scope = folder
RDX->>FS: Save folder with requestTabOrder
else Scope = collection
RDX->>FS: Save collection with requestTabOrder
end
RDX->>UI: State updated → tabs re-render in new order
sequenceDiagram
participant User
participant UI as Request Pane UI
participant Toggle as PaneToggles
participant RDX as Redux
participant Panel as RequestTabPanel
User->>Toggle: Click request/response toggle
Toggle->>RDX: Dispatch toggleRequestPane / toggleResponsePane (uid)
RDX->>RDX: Flip focusedTab.requestPaneVisible / responsePaneVisible
RDX->>Panel: Updated state
Panel->>Panel: Conditionally render panes and adjust layout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kanakkholwal
changed the title
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/bruno-schema/src/collections/index.js (1)
292-303: Apply lint suppression to allYup.when()configs in this file, not just the two specified lines.The Biome
noThenPropertyrule is active and will flag all 19Yup.when()occurrences withthenproperties throughout this file (lines 227–574), not just lines 292–303. The suggested inline comment syntax is correct, but limiting it to onlytokenHeaderPrefixandtokenQueryKeywon't keep CI green—the linter will still fail on the remaining 17 instances.Consider:
- Adding the suppression comment to all
Yup.when()configs in this file, or- Checking if a rule exception should be added globally to biome.json for this pattern, since Yup's schema validation legitimately requires the
thenproperty.packages/bruno-app/src/components/RequestPane/WsQueryUrl/index.js (1)
105-113: Remove debug console.log statement.Line 107 contains a debug statement that should be removed before merging.
🧹 Proposed fix
const handleUrlChange = (value) => { const finalUrl = value?.trim() ?? value; - console.log('finalUrl: ', finalUrl); dispatch(requestUrlChanged({ itemUid: item.uid, collectionUid: collection.uid, url: finalUrl })); };packages/bruno-app/src/components/RequestPane/GrpcQueryUrl/index.js (1)
279-279: Debounced function is recreated on every render.
debouncedOnUrlChangeat line 279 creates a new debounced function on each render, which resets the debounce timer and defeats the purpose of debouncing. Wrap it withuseMemooruseCallbackto preserve the same function instance across renders.🔧 Proposed fix
- const debouncedOnUrlChange = debounce(onUrlChange, 1000); + const debouncedOnUrlChange = useMemo( + () => debounce(onUrlChange, 1000), + [] + );Note: You may also want to add cleanup via
useEffectto cancel pending debounced calls on unmount:useEffect(() => { return () => { debouncedOnUrlChange.cancel(); }; }, [debouncedOnUrlChange]);Also applies to: 310-310
packages/bruno-app/src/components/RequestPane/WSRequestPane/index.js (1)
46-78: Includeitemin the dependency array, or extracteffectiveTabOrderto a separate memo.Line 78's dependency array includes only
item.uid, butgetEffectiveTabOrderat line 75 readsitem.typeanditem.requestTabOrder. If these properties change withoutitem.uidchanging, the memo won't recompute. Other request panes (HttpRequestPane, GraphQLRequestPane) handle this correctly by using[item, collection, preferences]for thegetEffectiveTabOrdercall, either in a separate memo or as a dependency.
🤖 Fix all issues with AI agents
In `@packages/bruno-app/src/components/RequestPane/GrpcRequestPane/index.js`:
- Around line 103-106: The memo that computes sorted tabs can miss changes to
item.requestTabOrder because its dependency list only includes item.uid; update
the logic to compute effectiveTabOrder in its own useMemo (like HttpRequestPane)
by calling getEffectiveTabOrder(item, collection, preferences) and include item
and item.requestTabOrder (or item as a whole) in that memo's dependency array,
then use that memoized effectiveTabOrder when calling sortTabs in the outer memo
so changes to item.requestTabOrder correctly trigger recomputation of tabs.
In `@packages/bruno-app/src/components/RequestPane/PaneToggles/index.js`:
- Around line 14-22: The handlers handleToggleRequestPane and
handleToggleResponsePane dispatch toggles using item.uid while the UI
color/state is read from focusedTab?.requestPaneVisible and
focusedTab?.responsePaneVisible, which can cause mismatches; update the handlers
to dispatch using focusedTab?.uid (or early-return if focusedTab is undefined)
when calling toggleRequestPane and toggleResponsePane, or add a guard that
verifies item.uid === focusedTab.uid before dispatching so the toggle always
targets the tab that the UI is reflecting.
In `@packages/bruno-app/src/providers/ReduxStore/slices/tabs.js`:
- Around line 172-182: The toggle handlers toggleRequestPane and
toggleResponsePane should treat missing visibility flags on migrated tabs as
currently visible; update each to compute currentVisible =
(tab.requestPaneVisible !== false) for request and (tab.responsePaneVisible !==
false) for response, then set the field to !currentVisible so undefined is
treated as true before inverting; locate these in the reducer operating on
state.tabs and replace the simple negation with this defensive defaulting logic.
In `@packages/bruno-app/src/ui/ResponsiveTabs/DraggableTab.js`:
- Around line 32-37: The code assumes monitor.getClientOffset() always returns
an object and dereferences .x which can be null; in the hover handler (the
function that computes hoverClientX using monitor.getClientOffset()) add a null
guard: call monitor.getClientOffset(), check for null and bail out (return) if
null before computing hoverClientX or using hoverBoundingRect, so hoverClientX =
clientOffset.x - hoverBoundingRect.left is only executed when clientOffset is
non-null.
In `@packages/bruno-app/src/ui/ResponsiveTabs/index.js`:
- Around line 206-233: The drag ordering is using the visible list index which
can differ from the underlying tabs order (e.g., when active tab is moved from
overflow), causing mis-reorders; update renderTab (and the other occurrences
noted) to compute the tab's original index in the full tabs array (e.g., const
originalIndex = tabs.findIndex(t => t.key === tab.key)) and pass that
originalIndex to DraggableTab's index prop and to moveTab so drag operations use
the canonical tabs indices instead of visibleTabKeys indices; ensure moveTab and
any drag handlers expect and operate on these full-array indices.
In `@packages/bruno-app/src/utils/tabs/index.js`:
- Around line 66-72: The current folder-scope branch returns
parentFolder.requestTabOrder directly which can be undefined and prevents
falling back to collection.requestTabOrder; update the logic in the scope ===
'folder' branch (where findParentItemInCollection(collection, item.uid) is
called) to return parentFolder.requestTabOrder when present otherwise fall back
to collection.requestTabOrder (e.g., use a nullish/coalescing check or explicit
undefined check on parentFolder.requestTabOrder) so folders with no saved order
inherit the collection order.
In `@packages/bruno-lang/v2/src/jsonToBru.js`:
- Around line 305-307: The template emits token_query_key whenever
tokenPlacement is not 'header', which can produce an empty field when
tokenPlacement is unset; update the conditional(s) around token_query_key (the
expression using auth?.oauth2?.tokenPlacement !== 'header') to only emit when
auth?.oauth2?.tokenPlacement === 'query' (or explicitly equals 'query'), and
apply the same change for the repeated occurrence at the later block (the lines
handling token_placement/token_header_prefix/token_query_key) so token_query_key
is only written when tokenPlacement is 'query'.
In `@packages/bruno-lang/v2/src/jsonToCollectionBru.js`:
- Around line 196-198: The template currently emits token_query_key even when
tokenPlacement is unset; update the emission conditions to only output
token_query_key when auth?.oauth2?.tokenPlacement is truthy and not 'header'
(e.g., check auth?.oauth2?.tokenPlacement && auth?.oauth2?.tokenPlacement !==
'header') so empty tokenPlacement doesn't produce an empty field; apply the same
change for the other occurrences noted (the blocks around
token_placement/token_header_prefix/token_query_key at the other two locations)
and keep the token_header_prefix branch unchanged (it should still require
tokenPlacement === 'header').
🧹 Nitpick comments (5)
packages/bruno-app/src/utils/tabs/index.js (1)
23-79: Add JSDoc for the new tab-order helpers.These are exported utilities; a short JSDoc block will make usage/shape clearer.
As per coding guidelines: Add JSDoc comments to abstractions for additional details.
✍️ Example JSDoc
+/** + * Sort tabs by a persisted order list. + * `@param` {Array<{ key: string }>} tabs + * `@param` {string[]} order + * `@returns` {Array<{ key: string }>} + */ export const sortTabs = (tabs, order) => { if (!order || !order.length) { return tabs; } @@ -}; +}; + +/** + * Resolve the effective tab order for an item based on scope. + * `@param` {object} item + * `@param` {object} collection + * `@param` {object} preferences + * `@returns` {string[] | undefined} + */ export const getEffectiveTabOrder = (item, collection, preferences) => {packages/bruno-filestore/src/formats/bru/index.ts (1)
40-48: Avoid emitting emptyrequestTabOrderblocks on round-trips.With the current defaults, requests that never set tab order will be rewritten with an empty
requestTabOrder. Consider persisting only when the field is present and non-empty.🛠️ Suggested change
settings: _.get(json, 'settings', {}), tags: _.get(json, 'meta.tags', []), - requestTabOrder: _.get(json, 'meta.requestTabOrder', []), request: { @@ - const transformedJson = { + const transformedJson = { type: requestType, name: _.get(json, 'meta.name'), @@ - } as any; + } as any; + + const requestTabOrder = _.get(json, 'meta.requestTabOrder'); + if (requestTabOrder && requestTabOrder.length) { + transformedJson.requestTabOrder = requestTabOrder; + } @@ const bruJson = { meta: { name: _.get(json, 'name'), type: type, seq: !_.isNaN(sequence) ? Number(sequence) : 1, - tags: _.get(json, 'tags', []), - requestTabOrder: _.get(json, 'requestTabOrder', []) + tags: _.get(json, 'tags', []) } } as any; + + const requestTabOrder = _.get(json, 'requestTabOrder'); + if (requestTabOrder && requestTabOrder.length) { + bruJson.meta.requestTabOrder = requestTabOrder; + }Also applies to: 140-148
packages/bruno-filestore/src/formats/bru/tests/tab-order.spec.js (1)
10-31: Make stringify tests assert round‑trip behaviour instead of substring checks.Substring matching can pass even if ordering is misplaced elsewhere in the output; round‑tripping asserts the persisted behaviour directly.
As per coding guidelines: Write behaviour-driven tests, not implementation-driven ones.
✅ Example update
- const bru = stringifyBruRequest(request); - expect(bru).toContain('requestTabOrder: ['); - expect(bru).toContain('params'); - expect(bru).toContain('headers'); - expect(bru).toContain('body'); + const bru = stringifyBruRequest(request); + const parsed = parseBruRequest(bru); + expect(parsed.requestTabOrder).toEqual(['params', 'headers', 'body']); @@ - const bru = stringifyBruCollection(collection); - expect(bru).toContain('requestTabOrder: ['); - expect(bru).toContain('headers'); - expect(bru).toContain('params'); + const bru = stringifyBruCollection(collection); + const parsed = parseBruCollection(bru); + expect(parsed.requestTabOrder).toEqual(['headers', 'params']);Also applies to: 56-69
packages/bruno-app/src/components/RequestPane/PaneToggles/index.js (1)
24-50: Consider adding an early return iffocusedTabis undefined.If there's no focused tab, the icons will render with potentially incorrect colors and the toggles may not work as expected.
Optional guard
+ if (!focusedTab) { + return null; + } + return ( <div className="flex items-center h-full ml-1 mr-2 gap-2 cursor-pointer">packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js (1)
2857-2917: Extract type mapping logic to reduce duplication.The type mapping at lines 2868-2871 is duplicated in
getEffectiveTabOrder(inpackages/bruno-app/src/utils/tabs/index.js). Consider extracting a shared utility likegetRequestTypeKey(item)to eliminate this duplication.The
_updateRequestTabOrderreducer correctly handlesitemUid: nullfor collection-level updates, so that's fine.Note that the save operations (lines 2909-2915) are dispatched without awaiting. While this isn't a bug, consider debouncing if rapid reordering could cause unnecessary filesystem writes.
packages/bruno-app/src/components/RequestPane/GrpcRequestPane/index.js
Outdated
Show resolved
Hide resolved
- Update tab visibility toggles and improve tab order handling
1 participant
Description
Feature implement to resolve #6949
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Preview
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.