fix(ws): persist message name rename on cmd+s/ctrl+s while editing#8447
Open
pooja-bruno wants to merge 1 commit into
Open
fix(ws): persist message name rename on cmd+s/ctrl+s while editing#8447pooja-bruno wants to merge 1 commit into
pooja-bruno wants to merge 1 commit into
Conversation
pooja-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno,
sid-bruno,
utkarsh-bruno and
vijayh-bruno
as code owners
Contributor
WalkthroughAdds a Ctrl/⌘+S keyboard shortcut in the WebSocket message name editor to prevent default browser save, commit the edited name, and dispatch a save action. Corresponding Playwright tests verify rename persistence for both .bru and .yml collection formats. ChangesWS Message Rename Shortcut
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SingleWSMessage
participant Redux
participant Filesystem
User->>SingleWSMessage: Double-click label, edit name
User->>SingleWSMessage: Press Ctrl/Cmd+S
SingleWSMessage->>SingleWSMessage: preventDefault()
SingleWSMessage->>SingleWSMessage: saveName(editValue)
SingleWSMessage->>Redux: dispatch saveRequest(item.uid, collection.uid)
Redux->>Filesystem: Persist renamed message to .bru/.yml
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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/RequestPane/WsBody/SingleWSMessage/index.js (1)
82-93: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winStop the bubbled Ctrl/⌘+S here
RequestTabs/RequestTabalso binds the save shortcut for the active tab, so this input can trigger bothsaveName(editValue)and the tab-leveldispatch(saveRequest(...)). Adde.stopPropagation()afterpreventDefault()to avoid a duplicate save/toast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bruno-app/src/components/RequestPane/WsBody/SingleWSMessage/index.js` around lines 82 - 93, The Ctrl/⌘+S handler in handleNameKeyDown currently lets the shortcut bubble up to RequestTabs/RequestTab, causing a duplicate save. Update the existing keydown logic to stop propagation in the same branch where preventDefault is called, so the SingleWSMessage name editor handles the shortcut without triggering the tab-level save dispatch.
🧹 Nitpick comments (2)
tests/websockets/multi-message-bru/multi-message.spec.ts (1)
257-282: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the test body in
test.stepfor readable reports.The rest of the coding standards for this test directory recommend promoting
test.stepusage. This new test has no step breakdown (rename, save, verify UI, verify persistence), unlike what's implied by the surrounding test suite conventions.As per path instructions, "Promote the use of
test.stepas much as possible so the generated reports are easier to read."♻️ Suggested structure
test('rename a message and save with the keyboard shortcut while editing', async ({ pageWithUserData: page }) => { const saveShortcut = process.platform === 'darwin' ? 'Meta+s' : 'Control+s'; - await openRequest(page, COLLECTION_NAME, SINGLE_MSG_REQ); - - const messageLabel = page.getByTestId('ws-message-label-0'); - await messageLabel.dblclick(); - - const nameInput = page.getByTestId('ws-message-name-input-0'); - await expect(nameInput).toBeVisible(); - - await nameInput.selectText(); - await page.keyboard.type('renamed via shortcut'); - - // Press cmd/ctrl+s while the name input still has focus, without pressing - // Enter or blurring first. This must commit the pending name and persist it. - await nameInput.press(saveShortcut); - - // The editing input closes and the new name is committed to the UI. - await expect(page.getByTestId('ws-message-label-0').filter({ hasText: 'renamed via shortcut' })).toBeVisible(); - - // The rename must be written to disk by the shortcut-triggered save. - await expect - .poll(async () => await readFile(SINGLE_MSG_BRU_PATH, 'utf8')) - .toContain('name: renamed via shortcut'); + await openRequest(page, COLLECTION_NAME, SINGLE_MSG_REQ); + + const messageLabel = page.getByTestId('ws-message-label-0'); + const nameInput = page.getByTestId('ws-message-name-input-0'); + + await test.step('Rename message and trigger save shortcut while editing', async () => { + await messageLabel.dblclick(); + await expect(nameInput).toBeVisible(); + await nameInput.selectText(); + await page.keyboard.type('renamed via shortcut'); + await nameInput.press(saveShortcut); + }); + + await test.step('Verify name is committed in the UI', async () => { + await expect(messageLabel.filter({ hasText: 'renamed via shortcut' })).toBeVisible(); + }); + + await test.step('Verify name is persisted to disk', async () => { + await expect + .poll(async () => await readFile(SINGLE_MSG_BRU_PATH, 'utf8')) + .toContain('name: renamed via shortcut'); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/websockets/multi-message-bru/multi-message.spec.ts` around lines 257 - 282, The new test "rename a message and save with the keyboard shortcut while editing" should be broken into `test.step` blocks for readability in reports. Refactor the test body to group the existing phases—opening the request, editing/renaming, triggering the save shortcut, verifying the UI update, and verifying persistence—using `test.step` inside the same test function so the scenario remains the same but the report shows clear sub-steps.Source: Path instructions
tests/websockets/multi-message-yml/multi-message.spec.ts (1)
316-319: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPersistence assertion is weaker than the
.brucounterpart.The
.brutest assertstoContain('name: renamed via shortcut')(verifying the actualnamefield), but this checks onlytoContain('renamed via shortcut'). A bug that wrote the value into the wrong field would still pass this assertion.🔧 Suggested tightening
await expect .poll(async () => await readFile(SINGLE_MSG_YML_PATH, 'utf8')) - .toContain('renamed via shortcut'); + .toContain('name: renamed via shortcut');Please confirm the exact YAML key/format used for message names before applying (e.g. quoting style may differ).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/websockets/multi-message-yml/multi-message.spec.ts` around lines 316 - 319, Tighten the persistence check in the multi-message websocket spec so it verifies the YAML name field rather than just any occurrence of the renamed text. Update the assertion in the shortcut-triggered save flow within the multi-message test to match the actual YAML key/format used for message names in this fixture, using the same style as the existing .bru test’s field-level check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@packages/bruno-app/src/components/RequestPane/WsBody/SingleWSMessage/index.js`:
- Around line 82-93: The Ctrl/⌘+S handler in handleNameKeyDown currently lets
the shortcut bubble up to RequestTabs/RequestTab, causing a duplicate save.
Update the existing keydown logic to stop propagation in the same branch where
preventDefault is called, so the SingleWSMessage name editor handles the
shortcut without triggering the tab-level save dispatch.
---
Nitpick comments:
In `@tests/websockets/multi-message-bru/multi-message.spec.ts`:
- Around line 257-282: The new test "rename a message and save with the keyboard
shortcut while editing" should be broken into `test.step` blocks for readability
in reports. Refactor the test body to group the existing phases—opening the
request, editing/renaming, triggering the save shortcut, verifying the UI
update, and verifying persistence—using `test.step` inside the same test
function so the scenario remains the same but the report shows clear sub-steps.
In `@tests/websockets/multi-message-yml/multi-message.spec.ts`:
- Around line 316-319: Tighten the persistence check in the multi-message
websocket spec so it verifies the YAML name field rather than just any
occurrence of the renamed text. Update the assertion in the shortcut-triggered
save flow within the multi-message test to match the actual YAML key/format used
for message names in this fixture, using the same style as the existing .bru
test’s field-level check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9d663151-d391-472f-b0f7-8e3d6dc1690b
📒 Files selected for processing (3)
packages/bruno-app/src/components/RequestPane/WsBody/SingleWSMessage/index.jstests/websockets/multi-message-bru/multi-message.spec.tstests/websockets/multi-message-yml/multi-message.spec.ts
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
1 participant
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
Previously, if you rename message title and then pressed cmd+s / ctrl+s while the name input was still focused, nothing was saved.
JIRA
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
Tests