Synced from GitHub
Thread transfer impact
⚠️ The latest CI run did not produce a thread transfer result for 970c434.
This comment will update automatically after the next completed run.
refactor(web): the add environment dialog owns its form state#22686
The add environment dialog's form state (mode, host, pairing code, SSH fields, suggestion popup, submitting flag), SSH host discovery, and submit handlers lived in ConnectionsSettings. Every keystroke in the dialog re-rendered the whole connections page. They now live in AddEnvironmentDialog.tsx. The page still owns the open flag, the route target, and the shared error, because the environment rows set them too (Add route, toggle and remove errors).
| File | Before | After |
|---|---|---|
| ConnectionsSettings.tsx | 2142 | 1538 |
| AddEnvironmentDialog.tsx | - | 649 |
Renders. Measured with ~/.cache/cvlayers/rc-init.js (counts only the fibers React rendered) on a dev server seeded by vp run migrate-dev-db, driven by agent-browser. Main is 6a6c84f990; after is the stack top, 1da6ebacee. Each cell is the median of 3 runs, shown as React commits / component renders.
This layer
| Interaction | main | after |
|---|---|---|
| open Connections | 8 / 325 | 8 / 326 |
| type 20 chars in Add environment host | 20 / 1880 | 20 / 620 |
Typing drops from 94 renders per keystroke to 31 (-67%). Opening the page costs 1 extra render, for the dialog component.
Whole stack (#22666 to #22686)
| Interaction | main | after |
|---|---|---|
| open General | 9 / 942 | 9 / 943 |
| toggle a setting (Rich text composer) | 1 / 103 | 1 / 104 |
| background activity dialog open + close | 39 / 441 | 39 / 385 |
| open Connections | 8 / 325 | 8 / 326 |
| type 20 chars in Add environment host | 20 / 1880 | 20 / 620 |
Tests run: tsc-rs --noEmit for apps/web, vp lint on changed files, vp test run src/reactCompilerCoverage.test.ts -t settings/ (0 bailouts), SettingsPanels.logic.test.ts, ConnectionsSettings.logic.test.ts.
🤖 Generated with Claude Code
Checks
- SuccessfulCodeRabbitGitHub
Review completed
- Successful
Skipped - Not affected
- Skipped
- Skipped
- Skipped
- SuccessfulRemove preview downloadGitHub
- SuccessfulCheckGitHub
- Skipped
- SkippedRustGitHub
- SkippedRelease SmokeGitHub
- SuccessfulTestGitHub
- SuccessfulBuild and TypecheckGitHub
- SuccessfulTest WebGitHub
- SuccessfulLabel PR 22686GitHub
- SkippedDeploy web previewGitHub
- Skipped
- SuccessfulLabel PR sizeGitHub
- Skipped
- Skipped
- SkippedEAS PreviewGitHub
- SuccessfulTest Server 5GitHub
- SuccessfulNative fingerprint diffGitHub
- SuccessfulTest Server 1GitHub
- SuccessfulTest Server 8GitHub
- SuccessfulTest Server 7GitHub
- SuccessfulTest Server 3GitHub
- SuccessfulTest Server 2GitHub
- SuccessfulTest Server 6GitHub
- SuccessfulChangesGitHub
- SuccessfulTest Server 4GitHub
- SuccessfulLintGitHub
- Skipped
- SuccessfulCollect PR targetsGitHub
- SuccessfulPrepare PR size configGitHub
Synced from GitHub
Synced from GitHub
Approvability
Verdict: Approved at 3960804
Macroscope's review found this PR approvable — This PR extracts the existing add-environment form into its own component and keeps the same connection, SSH discovery, relay-route, error, and default behavior. It is a contained structural refactor without production configuration, schema, or static-analysis changes.
No code changes detected at bd8e399. Prior analysis still applies.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
Important
Review skipped
Review was skipped as selected files did not have any reviewable changes.
⚙️ Run configuration
- Configuration used: Path: .coderabbit.config.ts
- Review profile: CHILL
- Plan: Team
- Run ID:
41d6c76f-c071-41ab-b78a-2b1093966d39
📥 Commits
Reviewing files that changed from the base of the PR and between 3960804af051b2dbbf44c91cb177d5be597f8aa1 and bd8e39916c28cf06c96f3c22ede04cf5ef08dff5.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
- 🔍 Trigger review
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
- Configuration used: Path: .coderabbit.config.ts
- Review profile: CHILL
- Plan: Team
- Run ID:
4da87a3a-baaa-431a-8790-fae788e3bc69
📥 Commits
Reviewing files that changed from the base of the PR and between 848e352b48f982e0f1fe134aa9712d75ba7da3b9 and 3960804af051b2dbbf44c91cb177d5be597f8aa1.
📒 Files selected for processing: 2
apps/web/src/components/settings/AddEnvironmentDialog.tsxapps/web/src/components/settings/ConnectionsSettings.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
📝 Walkthrough
The remote and SSH environment connection flows move from
ConnectionsSettingsinto the newAddEnvironmentDialogcomponent.ConnectionsSettingsretains dialog state and passes the component its route target, saved environments, and error callbacks.Changes: Environment Connection Dialog
Layer / File(s) Summary Remote and SSH connection flows
apps/web/src/components/settings/AddEnvironmentDialog.tsxThe new dialog handles remote pairing and desktop SSH connections. It includes SSH discovery and suggestions, validation, loading states, connection feedback, and route registration. Settings integration
apps/web/src/components/settings/ConnectionsSettings.tsxThe settings page removes its inline connection UI and related state and handlers. It renders AddEnvironmentDialogwith the dialog state, route target, saved environments, and error callbacks.Priority: ⬇️ Low
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Refactor
Suggested reviewers:
t3dotgg
Merge Risk: ⚪ Minimal · up to 39608
This is a behavior-preserving extraction of the add environment dialog's state into its own component. No regression was found, and the change looks safe to merge.
Pre-merge checks: Passed 3 · Failed 1
Failed Failed checks: 1 warning
Check name Status Explanation Resolution Description check Warning The description explains the problem, the code change, and verification results. It does not include the required Scope and approval section, and it does not provide the required before/after screensh… Add a Scope and approval section with the linked issue or maintainer approval, or explain why the change qualifies for an exemption. Add clear before/after screenshots and a recording if interaction timing or transitions require demonstrati… Passed Passed checks: 3 passed
Check name Status Explanation Title check Passed The title clearly and concisely describes the main change: moving add-environment form state into its dialog component. Linked Issues check Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check
Explanation
The description explains the problem, the code change, and verification results. It does not include the required Scope and approval section, and it does not provide the required before/after screenshots for this UI change.
Resolution
Add a Scope and approval section with the linked issue or maintainer approval, or explain why the change qualifies for an exemption. Add clear before/after screenshots and a recording if interaction timing or transitions require demonstration.
✨ Finishing Touches
📝 Generate docstrings
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Added 1 commit
- refactor(web): the add environment dialog owns its form statebd8e399