feat: revamp Runner UI with Timings and Filters sections#7505
Merged
bijin-bruno merged 9 commits intousebruno:mainfrom Mar 27, 2026
Merged
feat: revamp Runner UI with Timings and Filters sections#7505bijin-bruno merged 9 commits intousebruno:mainfrom
bijin-bruno merged 9 commits intousebruno:mainfrom
Conversation
gopu-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRemoves tag-enable radio toggles and configure-mode branching; always shows include/exclude tag lists, threads tags and selectedRequestItems/delay through run actions, disables requests by tag/transport in the configuration panel, adds a numeric Delay input, updates multiple styles, and adds Playwright tests and locators for the runner configuration UI. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "RunnerResults UI"
participant Panel as "RunConfigurationPanel"
participant Store as "Redux (runnerConfiguration)"
participant Runner as "Runner service / IPC"
UI->>Panel: render(tags, collection, savedConfig)
Panel->>Store: update selectedItems (on user select/reset/tags change)
Store-->>Panel: persisted runnerConfiguration
UI->>Store: updateRunnerConfiguration(selectedRequestItems, delay, tags)
Store->>Runner: runCollection/runCollectionFolder(payload with tags, selectedRequestItems, delay)
Runner-->>UI: runner started / status updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
packages/bruno-app/src/components/RunnerResults/index.jsx (1)
284-294: Adddata-testidfor Playwright testing.Same as the delay input in
RunCollectionItem- testable elements should havedata-testidattributes per coding guidelines.✨ Proposed fix
<input type="number" className="block textbox w-[60%]" placeholder="e.g. 5" autoComplete="off" autoCorrect="off" autoCapitalize="off" spellCheck="false" value={delay} onChange={(e) => setDelay(e.target.value)} + data-testid="runner-delay-input" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RunnerResults/index.jsx` around lines 284 - 294, In the RunnerResults component's delay input (the <input> that uses value={delay} and onChange={(e) => setDelay(e.target.value)}), add a data-testid attribute so Playwright can target it (use the same naming convention as RunCollectionItem's delay input, e.g. data-testid="run-delay-input"); update the JSX for that input in RunnerResults to include data-testid="run-delay-input".packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js (1)
84-94: Adddata-testidfor Playwright testing.Per coding guidelines, testable elements should have
data-testidattributes.✨ Proposed fix
<input type="number" className="textbox w-1/2" placeholder="e.g. 5" autoComplete="off" autoCorrect="off" autoCapitalize="off" spellCheck="false" value={delay} onChange={(e) => setDelay(e.target.value)} + data-testid="delay-input" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js` around lines 84 - 94, Add a data-testid attribute to the numeric input used for delay so Playwright tests can reliably target it: locate the input element in RunCollectionItem where props/state variables delay and setDelay are used and add a descriptive data-testid (e.g., data-testid="run-collection-delay-input") to the input element so tests can select it consistently.packages/bruno-app/src/components/RunnerResults/RunnerTags/index.jsx (1)
10-10: Consider memoizing the cloned collection.
cloneDeep(find(...))runs on every render. For large collections, this could impact performance. Consider usinguseMemoif profiling shows this as a bottleneck.✨ Potential optimization
+import { useMemo } from 'react'; ... - const collection = cloneDeep(find(collections, (c) => c.uid === collectionUid)); + const collection = useMemo( + () => cloneDeep(find(collections, (c) => c.uid === collectionUid)), + [collections, collectionUid] + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RunnerResults/RunnerTags/index.jsx` at line 10, The code repeatedly calls cloneDeep(find(collections, c => c.uid === collectionUid)) on every render; wrap this in useMemo to avoid unnecessary deep clones by computing it only when collections or collectionUid change (e.g., const collection = useMemo(() => cloneDeep(find(collections, c => c.uid === collectionUid)), [collections, collectionUid])); also add an import for useMemo from React if it's not already imported.packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/StyledWrapper.js (1)
11-77: Consider extracting shared input styles to reduce duplication.These styles (
.textbox,input[type='radio'],.single-line-editor,:has()container) are duplicated inRunnerResults/StyledWrapper.js. Consider extracting to a shared styled component or CSS mixin for maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/StyledWrapper.js` around lines 11 - 77, The styles for .textbox, div:has(> .single-line-editor), .single-line-editor and input[type='radio'] are duplicated in this StyledWrapper and RunnerResults/StyledWrapper.js; extract these shared rules into a reusable styled mixin or shared styled-component (e.g., SharedInputWrapper or inputStyles) and replace the duplicated blocks in both RunCollectionItem/StyledWrapper.js and RunnerResults/StyledWrapper.js with that shared import, keeping the exact selector names (.textbox, .single-line-editor, div:has(> .single-line-editor), input[type='radio']) so behavior and theming props remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/bruno-app/src/components/RunnerResults/index.jsx`:
- Around line 284-294: In the RunnerResults component's delay input (the <input>
that uses value={delay} and onChange={(e) => setDelay(e.target.value)}), add a
data-testid attribute so Playwright can target it (use the same naming
convention as RunCollectionItem's delay input, e.g.
data-testid="run-delay-input"); update the JSX for that input in RunnerResults
to include data-testid="run-delay-input".
In `@packages/bruno-app/src/components/RunnerResults/RunnerTags/index.jsx`:
- Line 10: The code repeatedly calls cloneDeep(find(collections, c => c.uid ===
collectionUid)) on every render; wrap this in useMemo to avoid unnecessary deep
clones by computing it only when collections or collectionUid change (e.g.,
const collection = useMemo(() => cloneDeep(find(collections, c => c.uid ===
collectionUid)), [collections, collectionUid])); also add an import for useMemo
from React if it's not already imported.
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js`:
- Around line 84-94: Add a data-testid attribute to the numeric input used for
delay so Playwright tests can reliably target it: locate the input element in
RunCollectionItem where props/state variables delay and setDelay are used and
add a descriptive data-testid (e.g., data-testid="run-collection-delay-input")
to the input element so tests can select it consistently.
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/StyledWrapper.js`:
- Around line 11-77: The styles for .textbox, div:has(> .single-line-editor),
.single-line-editor and input[type='radio'] are duplicated in this StyledWrapper
and RunnerResults/StyledWrapper.js; extract these shared rules into a reusable
styled mixin or shared styled-component (e.g., SharedInputWrapper or
inputStyles) and replace the duplicated blocks in both
RunCollectionItem/StyledWrapper.js and RunnerResults/StyledWrapper.js with that
shared import, keeping the exact selector names (.textbox, .single-line-editor,
div:has(> .single-line-editor), input[type='radio']) so behavior and theming
props remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 25f015b3-928b-4deb-89fd-ffaaebf32bbe
📒 Files selected for processing (6)
packages/bruno-app/src/components/RunnerResults/RunnerTags/index.jsxpackages/bruno-app/src/components/RunnerResults/StyledWrapper.jspackages/bruno-app/src/components/RunnerResults/index.jsxpackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.jspackages/bruno-app/src/components/TagList/index.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/bruno-app/src/components/RunnerResults/RunnerTags/index.jsx (2)
22-24: Minor: useEffect dependency uses derivedcollection.uidinstead of prop.The
collectionUidprop is already available and equivalent. Using the prop directly is cleaner and avoids potential issues ifcollectionis undefined.♻️ Suggested fix
useEffect(() => { dispatch(updateCollectionTagsList({ collectionUid })); - }, [collection.uid, dispatch]); + }, [collectionUid, dispatch]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RunnerResults/RunnerTags/index.jsx` around lines 22 - 24, The useEffect currently depends on derived collection.uid which can be undefined; change its dependency to the prop collectionUid instead: update the effect that calls dispatch(updateCollectionTagsList({ collectionUid })) so the dependency array uses [collectionUid, dispatch] (remove collection.uid) to ensure the effect tracks the prop directly; keep the same dispatch call and function name (updateCollectionTagsList) unchanged.
10-10: UnnecessarycloneDeepon every render.The collection is only used for reading
runnerTags,runnerTagsEnabled, andallTags. There's no mutation, so deep cloning on every render is wasteful.♻️ Suggested fix
- const collection = cloneDeep(find(collections, (c) => c.uid === collectionUid)); + const collection = find(collections, (c) => c.uid === collectionUid);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RunnerResults/RunnerTags/index.jsx` at line 10, Remove the unnecessary deep clone by replacing the cloneDeep(find(...)) usage: directly assign the result of find(collections, c => c.uid === collectionUid) to the collection variable (used only for reading runnerTags, runnerTagsEnabled, and allTags) and ensure no subsequent code mutates collection; keep cloneDeep only if later mutation is introduced. Update the declaration that currently uses cloneDeep to simply use find(collections, (c) => c.uid === collectionUid) and verify all usages of collection (runnerTags, runnerTagsEnabled, allTags) are read-only.packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js (1)
81-95: Consider addingmin="0"to prevent negative delay values.Users could enter negative numbers, which would be passed as-is to the runner. Adding a minimum constraint improves UX.
♻️ Suggested enhancement
<input type="number" className="textbox w-1/2" placeholder="e.g. 5" autoComplete="off" autoCorrect="off" autoCapitalize="off" spellCheck="false" + min="0" value={delay} onChange={(e) => setDelay(e.target.value)} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js` around lines 81 - 95, The numeric input for request delay (value bound to delay and updated via setDelay in the RunCollectionItem component) allows negative values; add an HTML min constraint to the input (min="0") and ensure onChange still parses/validates the value before calling setDelay so negative inputs cannot be set programmatically or via the UI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-app/src/components/RunnerResults/StyledWrapper.js`:
- Around line 30-43: The custom radio style removes native focus hints via
appearance: none, so add an explicit keyboard focus indicator by implementing a
:focus-visible rule on the same selector (input[type='radio']) to show a clear
outline or box-shadow using the theme (e.g., theme.primary.solid or a
semi-transparent variant) and ensure it does not affect mouse focus; update the
StyledWrapper input[type='radio'] block to include &:focus-visible { ... } with
a visible outline/box-shadow and appropriate border color so keyboard users can
perceive focus.
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js`:
- Line 17: The useState for delay in RunCollectionItem currently initializes
delay to null which causes React controlled/uncontrolled warnings when used as
value={delay}; change the initialization in the RunCollectionItem component's
const [delay, setDelay] = useState(null) to use an empty string (useState(''))
so the input is controlled from mount, and ensure any places that parse or
setDelay convert to/from string/number as needed (e.g., when submitting or
validating).
---
Nitpick comments:
In `@packages/bruno-app/src/components/RunnerResults/RunnerTags/index.jsx`:
- Around line 22-24: The useEffect currently depends on derived collection.uid
which can be undefined; change its dependency to the prop collectionUid instead:
update the effect that calls dispatch(updateCollectionTagsList({ collectionUid
})) so the dependency array uses [collectionUid, dispatch] (remove
collection.uid) to ensure the effect tracks the prop directly; keep the same
dispatch call and function name (updateCollectionTagsList) unchanged.
- Line 10: Remove the unnecessary deep clone by replacing the
cloneDeep(find(...)) usage: directly assign the result of find(collections, c =>
c.uid === collectionUid) to the collection variable (used only for reading
runnerTags, runnerTagsEnabled, and allTags) and ensure no subsequent code
mutates collection; keep cloneDeep only if later mutation is introduced. Update
the declaration that currently uses cloneDeep to simply use find(collections,
(c) => c.uid === collectionUid) and verify all usages of collection (runnerTags,
runnerTagsEnabled, allTags) are read-only.
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js`:
- Around line 81-95: The numeric input for request delay (value bound to delay
and updated via setDelay in the RunCollectionItem component) allows negative
values; add an HTML min constraint to the input (min="0") and ensure onChange
still parses/validates the value before calling setDelay so negative inputs
cannot be set programmatically or via the UI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f00f5f5d-3f1f-4e40-8bad-6cdb43d1f01f
📒 Files selected for processing (4)
packages/bruno-app/src/components/RunnerResults/RunnerTags/index.jsxpackages/bruno-app/src/components/RunnerResults/StyledWrapper.jspackages/bruno-app/src/components/RunnerResults/index.jsxpackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-app/src/components/RunnerResults/index.jsx
...-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js`:
- Line 37: The dispatch call is passing Number(delay) unvalidated; normalize and
clamp delay before dispatching runCollectionFolder by parsing delay to an
integer (e.g., parseInt or Math.floor on Number(delay)), defaulting to 0 for
NaN, and ensuring it is non-negative (clamp negative values to 0) so invalid or
negative delays are never sent to IPC; apply the same normalization wherever
runCollectionFolder is dispatched in this file (including the other handler
around lines 87-96) and use the normalizedDelay variable in the dispatch call
instead of Number(delay).
- Around line 85-96: The label and input for the delay field in
RunCollectionItem are not associated for accessibility; add a unique id to the
input (e.g., "delay-input" or generated from collection data) and set the
label's htmlFor to that id so clicks focus the input and screen readers announce
correctly; update the input element that uses value={delay} and onChange={(e) =>
setDelay(e.target.value)} to include id="your-chosen-id" and update the label to
htmlFor="your-chosen-id".
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/StyledWrapper.js`:
- Around line 74-88: The custom radio reset in StyledWrapper.js removes the
native focus indicator for the selector input[type='radio']; add a visible
keyboard focus state by targeting input[type='radio']:focus-visible (and/or
:focus) and applying a high-contrast outline or focus ring (e.g., outline or
box-shadow using props.theme.primary.solid or a contrast color) that is distinct
from the checked state; ensure the focus style works with the existing &:checked
rules and respects theme colors so keyboard users can see focus without changing
the visual design.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 792d2661-1be6-4712-bf41-31fd9c98befe
📒 Files selected for processing (2)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js
| ); | ||
| if (!isCollectionRunInProgress) { | ||
| dispatch(runCollectionFolder(collection.uid, item ? item.uid : null, recursive, 0, tagsEnabled && tags)); | ||
| dispatch(runCollectionFolder(collection.uid, item ? item.uid : null, recursive, Number(delay), tagsEnabled && tags)); |
Contributor
There was a problem hiding this comment.
Normalize delay before dispatching the run action.
Line 37 sends Number(delay) directly; invalid or negative values can be forwarded to IPC unguarded.
🛠️ Suggested fix
const onSubmit = (recursive) => {
+ const parsedDelay = Number.parseInt(delay, 10);
+ const delayMs = Number.isFinite(parsedDelay) && parsedDelay >= 0 ? parsedDelay : 0;
+
dispatch(
addTab({
uid: uuid(),
collectionUid: collection.uid,
type: 'collection-runner'
})
);
if (!isCollectionRunInProgress) {
- dispatch(runCollectionFolder(collection.uid, item ? item.uid : null, recursive, Number(delay), tagsEnabled && tags));
+ dispatch(runCollectionFolder(collection.uid, item ? item.uid : null, recursive, delayMs, tagsEnabled && tags));
}
onClose();
};
@@
<input
type="number"
className="textbox w-1/2"
placeholder="e.g. 5"
+ min="0"
+ step="1"
autoComplete="off"
autoCorrect="off"
autoCapitalize="off"
spellCheck="false"
value={delay}
onChange={(e) => setDelay(e.target.value)}
/>Also applies to: 87-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js`
at line 37, The dispatch call is passing Number(delay) unvalidated; normalize
and clamp delay before dispatching runCollectionFolder by parsing delay to an
integer (e.g., parseInt or Math.floor on Number(delay)), defaulting to 0 for
NaN, and ensuring it is non-negative (clamp negative values to 0) so invalid or
negative delays are never sent to IPC; apply the same normalization wherever
runCollectionFolder is dispatched in this file (including the other handler
around lines 87-96) and use the normalizedDelay variable in the dispatch call
instead of Number(delay).
...-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js
Outdated
Show resolved
Hide resolved
Comment on lines
+74
to
+88
| input[type='radio'] { | ||
| cursor: pointer; | ||
| appearance: none; | ||
| width: 16px; | ||
| height: 16px; | ||
| border-radius: 50%; | ||
| border: 1px solid ${(props) => props.theme.input.border}; | ||
| background-color: ${(props) => props.theme.bg}; | ||
| flex-shrink: 0; | ||
|
|
||
| &:checked { | ||
| border: 1px solid ${(props) => props.theme.primary.solid}; | ||
| background-image: radial-gradient(circle, ${(props) => props.theme.primary.solid} 40%, ${(props) => props.theme.bg} 42%); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Add visible keyboard focus state for custom radios.
Line 74-88 removes native radio appearance, but no focus indicator is added back. Keyboard users may lose track of focus.
♿ Suggested fix
input[type='radio'] {
cursor: pointer;
appearance: none;
width: 16px;
height: 16px;
border-radius: 50%;
border: 1px solid ${(props) => props.theme.input.border};
background-color: ${(props) => props.theme.bg};
flex-shrink: 0;
+ &:focus-visible {
+ outline: 2px solid ${(props) => props.theme.primary.solid};
+ outline-offset: 2px;
+ }
+
&:checked {
border: 1px solid ${(props) => props.theme.primary.solid};
background-image: radial-gradient(circle, ${(props) => props.theme.primary.solid} 40%, ${(props) => props.theme.bg} 42%);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| input[type='radio'] { | |
| cursor: pointer; | |
| appearance: none; | |
| width: 16px; | |
| height: 16px; | |
| border-radius: 50%; | |
| border: 1px solid ${(props) => props.theme.input.border}; | |
| background-color: ${(props) => props.theme.bg}; | |
| flex-shrink: 0; | |
| &:checked { | |
| border: 1px solid ${(props) => props.theme.primary.solid}; | |
| background-image: radial-gradient(circle, ${(props) => props.theme.primary.solid} 40%, ${(props) => props.theme.bg} 42%); | |
| } | |
| } | |
| input[type='radio'] { | |
| cursor: pointer; | |
| appearance: none; | |
| width: 16px; | |
| height: 16px; | |
| border-radius: 50%; | |
| border: 1px solid ${(props) => props.theme.input.border}; | |
| background-color: ${(props) => props.theme.bg}; | |
| flex-shrink: 0; | |
| &:focus-visible { | |
| outline: 2px solid ${(props) => props.theme.primary.solid}; | |
| outline-offset: 2px; | |
| } | |
| &:checked { | |
| border: 1px solid ${(props) => props.theme.primary.solid}; | |
| background-image: radial-gradient(circle, ${(props) => props.theme.primary.solid} 40%, ${(props) => props.theme.bg} 42%); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/StyledWrapper.js`
around lines 74 - 88, The custom radio reset in StyledWrapper.js removes the
native focus indicator for the selector input[type='radio']; add a visible
keyboard focus state by targeting input[type='radio']:focus-visible (and/or
:focus) and applying a high-contrast outline or focus ring (e.g., outline or
box-shadow using props.theme.primary.solid or a contrast color) that is distinct
from the checked state; ensure the focus style works with the existing &:checked
rules and respects theme colors so keyboard users can see focus without changing
the visual design.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js (1)
81-98: Addminattribute to prevent negative delay input.The number input accepts negative values, which aren't meaningful for a delay. Adding
min="0"provides clear UI feedback and aligns with the backend expectation.♻️ Proposed fix
<input id="runner-delay" type="number" className="textbox w-1/2" placeholder="e.g. 5" + min="0" autoComplete="off" autoCorrect="off" autoCapitalize="off" spellCheck="false" value={delay} onChange={(e) => setDelay(e.target.value)} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js` around lines 81 - 98, The number input in the RunCollectionItem component allows negative values for the delay (value and setter: delay, setDelay on the input with id "runner-delay"); update the input element to include a min="0" attribute to prevent negative numbers at the UI level and ensure user input aligns with backend expectations, keeping the existing value and onChange handler intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js`:
- Around line 81-98: The number input in the RunCollectionItem component allows
negative values for the delay (value and setter: delay, setDelay on the input
with id "runner-delay"); update the input element to include a min="0" attribute
to prevent negative numbers at the UI level and ensure user input aligns with
backend expectations, keeping the existing value and onChange handler intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2de25581-ec5c-4d64-b2df-23cc3fad73dc
📒 Files selected for processing (3)
packages/bruno-app/src/components/RunnerResults/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/StyledWrapper.jspackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.js
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/bruno-app/src/components/RunnerResults/StyledWrapper.js
- packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/StyledWrapper.js
gopu-bruno
force-pushed
the
feat/runner-ui-revamp
branch
from
a732b17 to
58761bd
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/runner/runner-configuration.spec.ts (1)
22-156: Add one modal-path test forRunCollectionItem.These cases only exercise the main Runner tab. The collection/folder modal got its own timings and filters UI in this PR, so regressions there would still slip through.
As per coding guidelines, "Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/runner/runner-configuration.spec.ts` around lines 22 - 156, The test suite is missing a modal-path test for RunCollectionItem that exercises the collection/folder modal UI (timings and filters); add a new test in this file that uses buildRunnerLocators, opens the Runner with openRunnerTab(COLLECTION_NAME), then triggers the RunCollectionItem modal (e.g., locate and click the collection/folder run button or RunCollectionItem trigger), wait for the modal to appear, and assert the modal’s timing inputs and filter controls are visible and interactive (use waitForRequestsInitialized where appropriate and locators similar to locators.configPanel(), locators.configCounter(), and locators.configResetButton() to verify state and behavior). Ensure the test toggles a timing/filter value and confirms the change persists or affects the counter/state, and include appropriate timeouts like the other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-app/src/components/RunnerResults/index.jsx`:
- Line 180: The call to updateRunnerConfiguration is overwriting the stored
requestItemsOrder by passing selectedRequestItems as both the selected list and
the order; change the third argument so you pass the full preserved order (e.g.,
requestItemsOrder or runnerConfig.requestItemsOrder) instead of
selectedRequestItems so disabled/unselected items keep their original positions
when you dispatch(updateRunnerConfiguration(collection.uid,
selectedRequestItems, /* use existing requestItemsOrder here */, delay)); ensure
you reference the component or store variable that holds the current full order
(requestItemsOrder) rather than the selected subset.
- Around line 269-275: Add a stable Playwright locator by adding a data-testid
attribute (e.g. data-testid="runner-run-button") to the Button in the
RunnerResults component (the Button rendering "Run {selectedRequestItems.length}
Request(s)"), and update the test helper runCollectionButton in
tests/utils/page/runner.ts to use page.getByTestId('runner-run-button') instead
of looking up the text "Run Collection" so e2e flows use the stable locator.
- Line 181: Sanitize the delay before dispatching runCollectionFolder by parsing
delay/savedDelay once into a non-negative integer and reusing that value for
persistence and execution: compute sanitizedDelay = Math.max(0,
parseInt(delayOrSavedDelay, 10)) (or 0 if NaN) and pass sanitizedDelay to
dispatch(runCollectionFolder(collection.uid, null, true, sanitizedDelay, tags,
selectedRequestItems)) and to any save/persist calls; update references around
the dispatch and the similar block at lines 189-197 to use the same sanitized
variable instead of Number(delay)/Number(savedDelay).
In
`@packages/bruno-app/src/components/RunnerResults/RunConfigurationPanel/index.jsx`:
- Around line 283-289: The dispatch calls that invoke updateRunnerConfiguration
(seen near setSelectedItems and where selection/order changes) replace the
entire runnerConfiguration and thus drop any saved delay; fix by threading the
current delay through these calls or updating the reducer to merge fields
instead of replacing: when calling updateRunnerConfiguration(collection.uid,
ordered, allRequestUidsOrder) include the current delay value (e.g., pull
runnerConfiguration.delay from props/state) so the action carries delay along
with selection/order, or change the reducer handling updateRunnerConfiguration
to merge incoming selection/order into existing runnerConfiguration (preserving
delay) rather than overwriting the whole object.
---
Nitpick comments:
In `@tests/runner/runner-configuration.spec.ts`:
- Around line 22-156: The test suite is missing a modal-path test for
RunCollectionItem that exercises the collection/folder modal UI (timings and
filters); add a new test in this file that uses buildRunnerLocators, opens the
Runner with openRunnerTab(COLLECTION_NAME), then triggers the RunCollectionItem
modal (e.g., locate and click the collection/folder run button or
RunCollectionItem trigger), wait for the modal to appear, and assert the modal’s
timing inputs and filter controls are visible and interactive (use
waitForRequestsInitialized where appropriate and locators similar to
locators.configPanel(), locators.configCounter(), and
locators.configResetButton() to verify state and behavior). Ensure the test
toggles a timing/filter value and confirms the change persists or affects the
counter/state, and include appropriate timeouts like the other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f59e5614-4509-4f37-bf9e-5f1f2fc4cf0e
📒 Files selected for processing (9)
packages/bruno-app/src/components/RunnerResults/RunConfigurationPanel/StyledWrapper.jsxpackages/bruno-app/src/components/RunnerResults/RunConfigurationPanel/index.jsxpackages/bruno-app/src/components/RunnerResults/RunnerTags/index.jsxpackages/bruno-app/src/components/RunnerResults/StyledWrapper.jspackages/bruno-app/src/components/RunnerResults/index.jsxpackages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RunCollectionItem/index.jspackages/bruno-app/src/components/TagList/StyledWrapper.jstests/runner/runner-configuration.spec.tstests/utils/page/runner.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-app/src/components/RunnerResults/StyledWrapper.js
| dispatch(runCollectionFolder(collection.uid, null, true, Number(delay), tagsEnabled && tags)); | ||
| } | ||
| dispatch(updateRunnerConfiguration(collection.uid, selectedRequestItems, selectedRequestItems, delay)); | ||
| dispatch(runCollectionFolder(collection.uid, null, true, Number(delay), tags, selectedRequestItems)); |
Contributor
There was a problem hiding this comment.
Sanitize delay before dispatching the run.
Number(delay) / Number(savedDelay) can forward NaN, negatives, or fractional values straight to IPC. Parse once to a non-negative integer and reuse that value for both persistence and execution.
🛠️ Suggested fix
+const normalizeDelay = (value) => {
+ const parsed = Number(value);
+ return Number.isFinite(parsed) && parsed >= 0 ? Math.floor(parsed) : 0;
+};
+
const runCollection = () => {
- dispatch(updateRunnerConfiguration(collection.uid, selectedRequestItems, selectedRequestItems, delay));
- dispatch(runCollectionFolder(collection.uid, null, true, Number(delay), tags, selectedRequestItems));
+ const delayMs = normalizeDelay(delay);
+ dispatch(updateRunnerConfiguration(collection.uid, selectedRequestItems, selectedRequestItems, delayMs));
+ dispatch(runCollectionFolder(collection.uid, null, true, delayMs, tags, selectedRequestItems));
};
const runAgain = () => {
ensureCollectionIsMounted();
// Get the saved configuration to determine what to run
const savedConfiguration = get(collection, 'runnerConfiguration', null);
const savedSelectedItems = savedConfiguration?.selectedRequestItems || [];
const savedDelay = savedConfiguration?.delay !== undefined ? savedConfiguration.delay : delay;
+ const delayMs = normalizeDelay(savedDelay);
dispatch(
runCollectionFolder(
collection.uid,
runnerInfo.folderUid,
true,
- Number(savedDelay),
+ delayMs,
tags,
savedSelectedItems
)
);
};Also applies to: 189-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-app/src/components/RunnerResults/index.jsx` at line 181,
Sanitize the delay before dispatching runCollectionFolder by parsing
delay/savedDelay once into a non-negative integer and reusing that value for
persistence and execution: compute sanitizedDelay = Math.max(0,
parseInt(delayOrSavedDelay, 10)) (or 0 if NaN) and pass sanitizedDelay to
dispatch(runCollectionFolder(collection.uid, null, true, sanitizedDelay, tags,
selectedRequestItems)) and to any save/persist calls; update references around
the dispatch and the similar block at lines 189-197 to use the same sanitized
variable instead of Number(delay)/Number(savedDelay).
Comment on lines
+283
to
+289
| if (changed) { | ||
| const ordered = flattenedRequests | ||
| .filter((r) => newSelected.includes(r.uid)) | ||
| .map((r) => r.uid); | ||
| setSelectedItems(ordered); | ||
| const allRequestUidsOrder = flattenedRequests.map((item) => item.uid); | ||
| dispatch(updateRunnerConfiguration(collection.uid, ordered, allRequestUidsOrder)); |
Contributor
There was a problem hiding this comment.
These config updates drop the saved delay.
updateRunnerConfiguration() replaces the whole runnerConfiguration object. The new tag-sync/select-all/reset dispatches only send selection and order, so any previously saved delay is cleared as soon as one of these handlers runs. Please thread the live delay through here, or merge config fields in the reducer instead of replacing them.
Also applies to: 359-367, 376-384
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/bruno-app/src/components/RunnerResults/RunConfigurationPanel/index.jsx`
around lines 283 - 289, The dispatch calls that invoke updateRunnerConfiguration
(seen near setSelectedItems and where selection/order changes) replace the
entire runnerConfiguration and thus drop any saved delay; fix by threading the
current delay through these calls or updating the reducer to merge fields
instead of replacing: when calling updateRunnerConfiguration(collection.uid,
ordered, allRequestUidsOrder) include the current delay value (e.g., pull
runnerConfiguration.delay from props/state) so the action carries delay along
with selection/order, or change the reducer handling updateRunnerConfiguration
to merge incoming selection/order into existing runnerConfiguration (preserving
delay) rather than overwriting the whole object.
bijin-bruno
approved these changes
Mar 27, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Revamps the Collection Runner UI (main Runner tab and Run Collection modal) with theme-based styling and two separate sections.
Part of : JIRA, JIRA
Screenshots
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.
Summary by CodeRabbit
New Features
Improvements
Tests