Skip to content

fix: restore cursor focus on save and show placeholder for empty cells#6795

Merged
sid-bruno merged 11 commits intousebruno:mainfrom
pooja-bruno:fix/editable-table-focus-and-placeholder
Jan 31, 2026
Merged

fix: restore cursor focus on save and show placeholder for empty cells#6795
sid-bruno merged 11 commits intousebruno:mainfrom
pooja-bruno:fix/editable-table-focus-and-placeholder

Conversation

@pooja-bruno
Copy link
Collaborator

@pooja-bruno pooja-bruno commented Jan 13, 2026

Description

  • Fix cursor disappearing after saving a request in EditableTable
  • Fix placeholder not showing for empty value cells when name has a value

JIRA

Contribution Checklist:

  • I've used AI significantly to create this pull request
  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

Before:

Screen.Recording.2026-01-13.at.4.39.47.PM.mov

After:

Screen.Recording.2026-01-13.at.4.39.17.PM.mov

Summary by CodeRabbit

  • Refactor

    • Simplified placeholder logic across multiple table components (Headers, Params, Vars, Forms) from row-state-based to value-based checks for improved clarity.
    • Added UID preservation during data merges in Redux store to maintain data consistency.
  • Tests

    • Added test suite for table focus and placeholder behavior validation.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 13, 2026

Walkthrough

This PR systematically refactors editable table components across the application to determine placeholder visibility based on value presence rather than row position. It also introduces UID preservation utilities in the Redux store to maintain React key stability during data merges, and adds a test validating placeholder and focus behavior.

Changes

Cohort / File(s) Summary
Editable Table Core
packages/bruno-app/src/components/EditableTable/index.js, packages/bruno-app/src/components/CollectionSettings/Headers/index.js, packages/bruno-app/src/components/FolderSettings/Headers/index.js, packages/bruno-app/src/components/EnvironmentVariablesTable/index.js
Simplified render props by removing isLastEmptyRow and row parameters. Replaced placeholder logic from isLastEmptyRow ? 'Label' : '' to !value ? 'Label' : '' for determining placeholder visibility based on value presence.
Request Pane Tables
packages/bruno-app/src/components/RequestPane/{Assertions,FormUrlEncodedParams,MultipartFormParams,QueryParams,RequestHeaders,Vars/VarsTable}/index.js
Consistent refactoring across request pane components: removed isLastEmptyRow from render function signatures and switched to value-based placeholder logic throughout Value, Name, and Content-Type column renderers.
Response Example Tables
packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/{ResponseExampleFileBody,ResponseExampleFormUrlEncodedParams,ResponseExampleHeaders,ResponseExampleMultipartFormParams,ResponseExampleParams}/index.js, packages/bruno-app/src/components/ResponseExample/ResponseExampleResponsePane/ResponseExampleResponseHeaders/index.js
Applied same placeholder simplification pattern to all response example table components: removed isLastEmptyRow/row from render props and replaced with value-based placeholder conditions.
Redux Store UID Preservation
packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js
Added utilities (preserveUidsAtPaths, mergeRequestWithPreservedUids, mergeRootWithPreservedUids) and configuration constants (REQUEST_UID_PATHS, ROOT_UID_PATHS) to retain existing UIDs during data merges, maintaining React key stability across file-event handlers.
Folder Settings Variables
packages/bruno-app/src/components/FolderSettings/Vars/VarsTable/index.js, packages/bruno-app/src/components/CollectionSettings/Vars/VarsTable/index.js
Updated render signatures and placeholder logic to depend on value presence; placeholder now shows type-specific labels (e.g., "Value" or "Expr") only when value is empty.
Test Coverage
tests/editable-table/editable-table.spec.ts
New Playwright test validating EditableTable behavior: verifies focus retention after save and placeholder visibility for empty Value cells using CodeMirror selectors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • helloanoop
  • lohit-bruno
  • bijin-bruno

Poem

🎯 Placeholders now dance with value's beat,
No more rows counting empty seats,
UIDs preserved through merge's flow,
React keys steady—watch them glow! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically addresses the two main fixes: cursor focus restoration on save and placeholder visibility for empty cells.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Jan 13, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @tests/editable-table/editable-table.spec.ts:
- Line 36: The test uses a macOS-only shortcut by calling
page.keyboard.press('Meta+s'); update the keypress to the cross-platform form
'ControlOrMeta+s' so the spec (editable-table.spec.ts) triggers the same save
action on both Windows/Linux and macOS; locate the call to page.keyboard.press
in the test and replace the argument accordingly (keep the rest of the test
unchanged).
🧹 Nitpick comments (2)
tests/editable-table/editable-table.spec.ts (2)

9-48: Consider wrapping logical steps in test.step for better reports.

Per coding guidelines, test.step should be used to make generated reports easier to read. The test has clear logical phases that would benefit from this structure.

Proposed refactor
   test('Cursor focus restored after save and placeholder shown for empty value', async ({ page, createTmpDir }) => {
     const collectionName = 'test-editable-table';

-    // Create a new collection
-    await createCollection(page, collectionName, await createTmpDir());
+    await test.step('Create a new collection', async () => {
+      await createCollection(page, collectionName, await createTmpDir());
+    });

-    // Create a request
-    await createRequest(page, 'Test Request', collectionName, {
-      url: 'https://httpbin.org/get'
+    await test.step('Create a request', async () => {
+      await createRequest(page, 'Test Request', collectionName, {
+        url: 'https://httpbin.org/get'
+      });
     });

-    // Navigate to Params tab
-    await selectRequestPaneTab(page, 'Params');
+    await test.step('Navigate to Params tab and type in Name field', async () => {
+      await selectRequestPaneTab(page, 'Params');

-    // Find the Query params table
-    const queryTable = page.locator('table').first();
-    const firstRow = queryTable.locator('tbody tr').first();
+      const queryTable = page.locator('table').first();
+      const firstRow = queryTable.locator('tbody tr').first();

-    // Get the Name input (regular input)
-    const nameInput = firstRow.locator('input[type="text"]').first();
-    await nameInput.click();
-    await page.keyboard.type('testParam');
+      const nameInput = firstRow.locator('input[type="text"]').first();
+      await nameInput.click();
+      await page.keyboard.type('testParam');

-    // Verify input has focus before save
-    await expect(nameInput).toBeFocused();
+      await expect(nameInput).toBeFocused();
+    });

-    // Save the request
-    await page.keyboard.press('Meta+s');
+    await test.step('Save the request and verify focus is restored', async () => {
+      const queryTable = page.locator('table').first();
+      const firstRow = queryTable.locator('tbody tr').first();
+      const nameInput = firstRow.locator('input[type="text"]').first();

-    // Wait for save toast
-    await expect(page.getByText('Request saved successfully').last()).toBeVisible();
+      await page.keyboard.press('ControlOrMeta+s');
+      await expect(page.getByText('Request saved successfully').last()).toBeVisible();
+      await expect(nameInput).toBeFocused();
+    });

-    // Verify cursor focus is restored after save
-    await expect(nameInput).toBeFocused();
+    await test.step('Verify placeholder shows for empty Value field', async () => {
+      const queryTable = page.locator('table').first();
+      const firstRow = queryTable.locator('tbody tr').first();
+      const valueCell = firstRow.locator('[data-testid="column-value"]');
+      const placeholder = valueCell.locator('pre.CodeMirror-placeholder');
+      await expect(placeholder).toHaveText('Value');
+    });
-
-    // Verify placeholder shows for empty Value field
-    const valueCell = firstRow.locator('[data-testid="column-value"]');
-    const placeholder = valueCell.locator('pre.CodeMirror-placeholder');
-    await expect(placeholder).toHaveText('Value');
   });

24-24: Use the data-testid attribute on the EditableTable wrapper instead of the generic table locator.

The page.locator('table').first() selector is fragile. Since the EditableTable component renders a wrapper div with data-testid="editable-table", use page.getByTestId('editable-table').first() instead. For better specificity, consider passing different testId props to the Query and Path EditableTable instances in QueryParams (e.g., testId="query-params-table" and testId="path-params-table").

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 339e63e and df0d10c.

📒 Files selected for processing (1)
  • tests/editable-table/editable-table.spec.ts
🧰 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() not func ()
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/editable-table/editable-table.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 existing expect() playwright calls

  • Avoid using page.pause() in code

  • Use locator variables for locators

  • Avoid using test.only

  • Use multiple assertions

  • Promote the use of test.step as much as possible so the generated reports are easier to read

  • Ensure that the fixtures like the collections are nested inside the fixtures folder

    Fixture 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/editable-table/editable-table.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: Unit Tests
  • GitHub Check: CLI Tests
  • GitHub Check: SSL Tests - Windows
  • GitHub Check: SSL Tests - macOS
  • GitHub Check: SSL Tests - Linux
🔇 Additional comments (2)
tests/editable-table/editable-table.spec.ts (2)

1-2: LGTM!

Imports are clean and properly structured.


4-7: LGTM!

Good setup with test.describe block and afterEach cleanup to close all collections.

await expect(nameInput).toBeFocused();

// Save the request
await page.keyboard.press('Meta+s');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use cross-platform keyboard shortcut.

Meta+s only works on macOS. For cross-platform compatibility, use ControlOrMeta+s.

Proposed fix
-    await page.keyboard.press('Meta+s');
+    await page.keyboard.press('ControlOrMeta+s');
📝 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
await page.keyboard.press('Meta+s');
await page.keyboard.press('ControlOrMeta+s');
🤖 Prompt for AI Agents
In @tests/editable-table/editable-table.spec.ts at line 36, The test uses a
macOS-only shortcut by calling page.keyboard.press('Meta+s'); update the
keypress to the cross-platform form 'ControlOrMeta+s' so the spec
(editable-table.spec.ts) triggers the same save action on both Windows/Linux and
macOS; locate the call to page.keyboard.press in the test and replace the
argument accordingly (keep the rest of the test unchanged).

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@packages/bruno-app/src/components/MultiLineEditor/index.js`:
- Around line 13-14: The module-level lastFocusedEditor causes cross-table focus
leaks; update MultiLineEditor to accept a table-scoped identifier (e.g., prop
tableId) and namespace the stored focus record by tableId so each table has
isolated state. Replace the global lastFocusedEditor lookup/restore logic to
store and read keyed by `${tableId}:${rowIndex}:${columnKey}` (or keep a
per-table object keyed by tableId) and update any uses in focus restoration and
saving (references: lastFocusedEditor, MultiLineEditor) so focus is only
restored when the tableId matches the current instance.

In `@packages/bruno-app/src/components/SingleLineEditor/index.js`:
- Around line 13-14: lastFocusedEditor currently stores only rowIndex and
columnKey and is shared at module scope, causing focus to cross between
different table instances; modify SingleLineEditor to accept a tableId prop (or
derive a unique table identifier from context), include tableId in the stored
lastFocusedEditor object, and update all reads/writes/compare logic that
reference lastFocusedEditor (e.g., matching logic that checks rowIndex and
columnKey) to also require tableId equality; ensure the module-level variable
structure is changed to include tableId and that callers (EditableTable) pass a
stable identifier so focus restoration is scoped per table instance.
🧹 Nitpick comments (2)
packages/bruno-app/src/components/MultiLineEditor/index.js (1)

152-160: Calling onEditorFocus on every cursor activity may be excessive.

_onCursorActivity invokes onEditorFocus on every cursor movement while the editor has focus. This could trigger many callbacks during normal typing/navigation. If onEditorFocus performs any non-trivial work upstream, this may impact performance.

If the intent is purely to track cursor position, consider a dedicated prop like onCursorChange to distinguish from focus events.

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

121-133: Auto-focus restoration looks correct but lacks boundary checks.

When rowIndex or columnKey are undefined, the equality check lastFocusedEditor.rowIndex === this.props.rowIndex would match any editor with undefined props, potentially causing unintended focus.

Consider adding explicit checks:

Suggested improvement
-    if (lastFocusedEditor
-      && lastFocusedEditor.rowIndex === this.props.rowIndex
-      && lastFocusedEditor.columnKey === this.props.columnKey) {
+    if (lastFocusedEditor
+      && this.props.rowIndex !== undefined
+      && this.props.columnKey !== undefined
+      && lastFocusedEditor.rowIndex === this.props.rowIndex
+      && lastFocusedEditor.columnKey === this.props.columnKey) {
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 935dd4c and abf331f.

📒 Files selected for processing (19)
  • packages/bruno-app/src/components/CollectionSettings/Headers/index.js
  • packages/bruno-app/src/components/CollectionSettings/Vars/VarsTable/index.js
  • packages/bruno-app/src/components/EditableTable/index.js
  • packages/bruno-app/src/components/FolderSettings/Headers/index.js
  • packages/bruno-app/src/components/FolderSettings/Vars/VarsTable/index.js
  • packages/bruno-app/src/components/MultiLineEditor/index.js
  • packages/bruno-app/src/components/RequestPane/Assertions/index.js
  • packages/bruno-app/src/components/RequestPane/FormUrlEncodedParams/index.js
  • packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js
  • packages/bruno-app/src/components/RequestPane/QueryParams/index.js
  • packages/bruno-app/src/components/RequestPane/RequestHeaders/index.js
  • packages/bruno-app/src/components/RequestPane/Vars/VarsTable/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleFileBody/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleFormUrlEncodedParams/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleHeaders/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleMultipartFormParams/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleParams/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleResponsePane/ResponseExampleResponseHeaders/index.js
  • packages/bruno-app/src/components/SingleLineEditor/index.js
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleResponsePane/ResponseExampleResponseHeaders/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleHeaders/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleMultipartFormParams/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleFormUrlEncodedParams/index.js
  • packages/bruno-app/src/components/RequestPane/Vars/VarsTable/index.js
  • packages/bruno-app/src/components/CollectionSettings/Headers/index.js
  • packages/bruno-app/src/components/CollectionSettings/Vars/VarsTable/index.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() not func ()
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/FolderSettings/Headers/index.js
  • packages/bruno-app/src/components/SingleLineEditor/index.js
  • packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js
  • packages/bruno-app/src/components/RequestPane/RequestHeaders/index.js
  • packages/bruno-app/src/components/MultiLineEditor/index.js
  • packages/bruno-app/src/components/RequestPane/FormUrlEncodedParams/index.js
  • packages/bruno-app/src/components/FolderSettings/Vars/VarsTable/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleFileBody/index.js
  • packages/bruno-app/src/components/RequestPane/Assertions/index.js
  • packages/bruno-app/src/components/EditableTable/index.js
  • packages/bruno-app/src/components/RequestPane/QueryParams/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleParams/index.js
🧠 Learnings (3)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.

Applied to files:

  • packages/bruno-app/src/components/FolderSettings/Headers/index.js
  • packages/bruno-app/src/components/SingleLineEditor/index.js
  • packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js
  • packages/bruno-app/src/components/RequestPane/RequestHeaders/index.js
  • packages/bruno-app/src/components/MultiLineEditor/index.js
  • packages/bruno-app/src/components/RequestPane/FormUrlEncodedParams/index.js
  • packages/bruno-app/src/components/FolderSettings/Vars/VarsTable/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleFileBody/index.js
  • packages/bruno-app/src/components/RequestPane/Assertions/index.js
  • packages/bruno-app/src/components/EditableTable/index.js
  • packages/bruno-app/src/components/RequestPane/QueryParams/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleParams/index.js
📚 Learning: 2026-01-09T18:25:14.640Z
Learnt from: kanakkholwal
Repo: usebruno/bruno PR: 6767
File: packages/bruno-app/src/components/ResponseExample/index.js:221-226
Timestamp: 2026-01-09T18:25:14.640Z
Learning: In the Bruno Electron renderer code (packages/bruno-app), assume window.ipcRenderer is always available and skip existence checks. Do not guard for ipcRenderer in this Electron context; use window.ipcRenderer directly (e.g., window.ipcRenderer.send(...), window.ipcRenderer.on(...)). If there are non-Electron contexts (such as test environments or non-Electron builds), add guards or mocks to avoid runtime errors there, but for the intended Electron renderer files, this pattern should be applied broadly within packages/bruno-app.

Applied to files:

  • packages/bruno-app/src/components/FolderSettings/Headers/index.js
  • packages/bruno-app/src/components/SingleLineEditor/index.js
  • packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js
  • packages/bruno-app/src/components/RequestPane/RequestHeaders/index.js
  • packages/bruno-app/src/components/MultiLineEditor/index.js
  • packages/bruno-app/src/components/RequestPane/FormUrlEncodedParams/index.js
  • packages/bruno-app/src/components/FolderSettings/Vars/VarsTable/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleFileBody/index.js
  • packages/bruno-app/src/components/RequestPane/Assertions/index.js
  • packages/bruno-app/src/components/EditableTable/index.js
  • packages/bruno-app/src/components/RequestPane/QueryParams/index.js
  • packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleParams/index.js
📚 Learning: 2025-12-02T20:48:05.670Z
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.670Z
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/EditableTable/index.js
🧬 Code graph analysis (2)
packages/bruno-app/src/components/FolderSettings/Headers/index.js (2)
packages/bruno-app/src/components/RequestPane/RequestHeaders/index.js (1)
  • headerAutoCompleteList (14-14)
packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleHeaders/index.js (1)
  • headerAutoCompleteList (13-13)
packages/bruno-app/src/components/MultiLineEditor/index.js (1)
packages/bruno-app/src/components/SingleLineEditor/index.js (1)
  • lastFocusedEditor (14-14)
⏰ 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 - macOS
  • GitHub Check: Playwright E2E Tests
  • GitHub Check: SSL Tests - Windows
  • GitHub Check: SSL Tests - Linux
  • GitHub Check: Unit Tests
  • GitHub Check: CLI Tests
🔇 Additional comments (25)
packages/bruno-app/src/components/RequestPane/RequestHeaders/index.js (2)

52-65: LGTM!

The render prop signature update and placeholder logic using showPlaceholder are correctly implemented. Passing rowIndex and columnKey to SingleLineEditor enables proper focus restoration across remounts, addressing the cursor disappearing issue mentioned in the PR objectives.


72-85: LGTM!

Value column changes are consistent with the Name column pattern. The showPlaceholder logic correctly replaces the previous isLastEmptyRow approach, fixing the placeholder visibility for empty value cells.

packages/bruno-app/src/components/RequestPane/FormUrlEncodedParams/index.js (1)

50-63: LGTM!

The render prop signature and placeholder logic are correctly updated for MultiLineEditor. The rowIndex and columnKey props enable focus restoration in the multi-line editor context.

packages/bruno-app/src/components/RequestPane/Assertions/index.js (1)

128-149: LGTM!

The Value column correctly incorporates both row (needed for parseAssertionOperator) and the new props (showPlaceholder, rowIndex, columnKey). The conditional rendering for unary operators is preserved, and focus tracking props are properly passed to SingleLineEditor.

packages/bruno-app/src/components/FolderSettings/Headers/index.js (2)

46-57: LGTM!

Name column render prop correctly updated with showPlaceholder, rowIndex, and columnKey. Consistent with the pattern established in RequestPane/RequestHeaders.


64-76: LGTM!

Value column changes mirror the Name column pattern correctly. Focus restoration and placeholder behavior will work as expected for folder-level headers.

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

103-122: Focus restoration logic looks correct.

The setTimeout(..., 0) ensures focus is deferred until after the DOM update completes. The null check on this.editor guards against race conditions if the component unmounts before the timeout fires.

One minor note: if rowIndex or columnKey are undefined (editor used outside EditableTable), the condition lastFocusedEditor.rowIndex === undefined would match any such editor. This is likely fine since those props wouldn't be passed in non-table contexts.


229-238: LGTM on cleanup!

The unmount logic properly:

  • Guards against null editor reference
  • Cleans up _destroyLinkAware if present
  • Removes all event listeners (change, focus, blur, cursorActivity)
  • Removes the wrapper element

This prevents memory leaks and stale event handlers.

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

168-194: Focus/cursor tracking handlers are well-implemented.

The _onFocus, _onBlur, and _onCursorActivity handlers correctly capture and propagate editor state. Good use of optional callback invocation with this.props.onEditorFocus and this.props.onEditorBlur.


255-257: Event listener cleanup is thorough.

Properly removing focus, blur, and cursorActivity listeners prevents memory leaks.

packages/bruno-app/src/components/RequestPane/QueryParams/index.js (2)

73-86: Query value column updates look good.

Render signature correctly updated to use showPlaceholder, rowIndex, and columnKey. Placeholder logic properly shows "Value" when showPlaceholder is true.


103-114: Path value column updates are consistent.

Same pattern applied correctly. Note that showPlaceholder is not destructured here but the placeholder is hardcoded as empty for path params (no placeholder prop passed), which is acceptable since path param names are auto-populated from the URL.

packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js (2)

125-176: Verify intentional use of both isLastEmptyRow and showPlaceholder.

The render signature now includes both isLastEmptyRow and showPlaceholder. Line 165 still uses isLastEmptyRow to hide the upload button on the last empty row, while showPlaceholder controls the placeholder text.

This appears intentional (different semantics), but verify this is the desired behavior — otherwise showPlaceholder should replace isLastEmptyRow entirely here as well.


183-193: Content-Type column updates are correct.

Render signature properly updated. showPlaceholder, rowIndex, and columnKey are passed to SingleLineEditor.

packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleFileBody/index.js (1)

133-146: Content-Type column updates are correct.

Render signature properly updated to use showPlaceholder, rowIndex, and columnKey. Placeholder logic correctly shows "Auto" when appropriate.

packages/bruno-app/src/components/ResponseExample/ResponseExampleRequestPane/ResponseExampleParams/index.js (3)

108-121: Query name column updates are correct.

Render signature properly updated. showPlaceholder, rowIndex, and columnKey correctly passed to SingleLineEditor.


130-143: Query value column updates are correct.

Consistent with the name column pattern.


161-174: Path value column updates are correct.

Same pattern applied consistently for path parameters.

packages/bruno-app/src/components/FolderSettings/Vars/VarsTable/index.js (1)

54-65: LGTM - Render prop signature updated for focus tracking.

The changes correctly implement the new render prop contract:

  • showPlaceholder is a cleaner semantic than isLastEmptyRow
  • rowIndex and columnKey enable MultiLineEditor to track and restore cursor position across remounts

MultiLineEditor stores the focused editor's position (rowIndex, columnKey, and cursor) in a module-level variable on focus, then restores focus and cursor when that same editor remounts, fixing the cursor disappearing issue.

packages/bruno-app/src/components/EditableTable/index.js (6)

25-33: Well-structured focus tracking ref.

Clean approach to track both standard inputs and CodeMirror editors with separate cursor position formats (cursorStart/cursorEnd vs cursorLine/cursorCh).


213-248: Good API design for custom column renderers.

The showPlaceholder flag and onEditorFocus/onEditorBlur callbacks provide a clean contract for custom editors to integrate with focus tracking. Passing columnKey and rowIndex enables proper identification for restoration.


277-287: Consider potential timing edge case with onBlur.

If the save operation causes the input to remount, the old element's onBlur fires and clears focusedInputRef before the restoration useEffect runs. This could prevent focus restoration in certain re-render scenarios.

If you've tested the save flow and it works correctly, this may not be an issue in practice (perhaps row UIDs remain stable). Otherwise, consider deferring the clear with a short timeout or tracking a "pending restore" flag.


288-294: Solid defensive check in onSelect.

The condition ensures cursor updates only apply to the currently tracked input, preventing interference from stale or out-of-order events.


262-262: Placeholder logic fix looks correct.

Using showPlaceholder (based on cell value emptiness) rather than row-level empty state ensures placeholders appear for any empty value cell, matching the PR objective.


79-100: Focus restoration only covers regular inputs, not CodeMirror editors.

This useEffect restores focus to input elements only. CodeMirror-based editors (SingleLineEditor/MultiLineEditor) handle their own focus restoration independently using a module-level lastFocusedEditor variable and restoring cursor position in their componentDidMount. The cursorLine and cursorCh values captured in focusedInputRef.current are stored but not applied by this effect—the editor components restore their own cursor state.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@sid-bruno sid-bruno merged commit 0c3d20b into usebruno:main Jan 31, 2026
7 of 8 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Feb 13, 2026
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants