Repository navigation
Say which device a settings section changes, and switch it right there - #1063
Merged
Merged
Conversation
sak0a
force-pushed
the
port/settings-scope-sentence
branch
from
October 31, 2026 09:51
0eae471 to
33d12d7
Compare
sak0a
force-pushed
the
port/settings-apply-to-all-nodes
branch
from
October 31, 2026 12:45
31b9854 to
05dc55d
Compare
sak0a
force-pushed
the
port/settings-scope-sentence
branch
from
October 31, 2026 12:56
33d12d7 to
4143cd0
Compare
- Node sections now start with "Changes apply to [Studio]". The device is a menu: the nav header's own device menu, with the same devices, the same rule for which ones a hosted browser may open, and the same switch. In hosted mode it moves the selection cursor, as before; it never opens a second target. - A device that is not reachable shows "Offline" or "Connecting", in the menu and in the sentence. - More than six devices get a search field. That list loads only when needed. - With two or more nodes known, sections with "Apply to all nodes" rows add "rows marked Mixed differ on other nodes". - This app's sections, and browser or device sections, keep their one-line label. The web phone tier is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n it never does Review fix: with more than six devices, the switch-device button opened nothing until the search list's chunk arrived, and a failed load threw to the root error view, taking Settings down with its only way to switch device. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sak0a
force-pushed
the
port/settings-scope-sentence
branch
from
October 31, 2026 14:11
4143cd0 to
efa02dd
Compare
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
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.
What Changed
Built on #1062 ("Apply to all nodes", merged).
SettingsNodeMenuwith one list (useSettingsNodeMenu). Both places show the same devices, apply the same rule for which ones a hosted browser may open, and switch through the same handler. The page no longer builds its own list.PhoneSettingsSurface) is untouched.docs/settings-ownership.mddescribes the sentence.Why
The device a section changes was a small grey label, and switching it meant finding the icon button at the bottom of the nav. With "Apply to all nodes" and "Mixed" on the same page, it matters to see which device you are editing, and to switch it where you read it. The page keeps one device list and one switch, so the two menus can never disagree.
Security note for review: the sentence is display plus the existing switch. Its menu takes its choices, its disabled rule and its handler from the same hook as the nav header, and the handler also refuses a device that is not in that list or that the list marks unavailable. So the scope menu can never pick a node the nav would not offer.
Validation
bun run checkandbun run check:changedon91125b136: everything passed except the same 5 server tests that fail onorigin/main(ProjectionPipeline.followUpQueue.test.ts,026_CanonicalizeModelSelectionOptions.test.ts; no server code here), and one markdown fuzz test that timed out under load and passes alone. The searchable list usespreloadableLazy, aswebBundleSplitting.test.tsrequires.SettingsPage.test.ts: the sentence is chosen by destination and section scope (node sections yes; client, browser and device sections no); the Mixed clause only on sections with fan-out rows; the Offline / Connecting rule.SettingsPageHosted.browser.tsx: a node section shows "Changes apply to" with a menu named after the device; picking Travel laptop moves the selection cursor, the sentence and the section description follow, and the section mounts again for the new node; the menu disables a node while the browser is not current, exactly as the nav does, and labels an offline node "Offline"; above six devices the list is searchable and picks through the cursor; Appearance shows no sentence. Checked against the S3SettingsPage.tsx: the three positive cases fail.SettingsPageSearchReveal,PhoneSettingsSurface.origin/mainbuild (95e814c): boot 523.5 KiB gzip both (unchanged), RootAppShell 398.0 → 397.5 KiB, settings route 186.9 → 192.7 KiB (S3 alone: 190.9). The searchable list (combobox and its vendor code, about 15 KiB) is its own chunk, loaded only above six devices.Review
An independent review checked that the sentence's menu and the nav header share one hook (same choices, same hosted gate). It also checked that hosted mode only moves the selection cursor and never adopts or reconnects a session itself, that offline or connecting devices cannot be written, the lazy search list, and the frozen phone tier (no diff). Fixed after it:
SettingsNodeMenu.browser.tsxmakes the load fail and still switches device; it fails without the fix.Left as they are (low):
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.