fix(web): pick the workspace in the account settings rail - #261
Open
isaiahknight-va wants to merge 1 commit into
Open
isaiahknight-va wants to merge 1 commit into
isaiahknight-va wants to merge 1 commit into
Conversation
The account settings modal rendered one rail group per workspace in the
order /api/workspaces returned, and it was never told which workspace the
user was standing in. ChatApp mounted it with the workspace list only, so
the first "Integrations" (and every other workspace section above the fold)
belonged to whichever workspace the API happened to list first. Clicking it
navigated to that other workspace's settings route, and "Close workspace
settings" then returned to that workspace's chat, silently moving the user
out of the workspace they started in. The per-group headings were the only
signal, and on a phone they are easy to miss.
The rail now carries one Workspace group holding one workspace's sections,
and a native select under the heading chooses whose. SettingsModal takes an
optional currentWorkspaceID; the select opens on that workspace (matched by
id or route id, falling back to the first in the list when it is unknown),
and that workspace's option text carries a "(current)" marker so the default
is legible in the open list too. Changing the select re-points the four
sections, so a second workspace's settings are two clicks away instead of a
scroll down a rail of repeated groups. A native select keeps keyboard and
screen-reader behavior for free, and a visually hidden label names it
"Workspace" for assistive technology. An account with one workspace keeps
the plain "Workspace · {name}" heading and gets no select. The preselection
lives in a pure helper, defaultSettingsWorkspace, so it is unit-testable
without Svelte.
The second half of the same flow: at phone widths the profile card that
opens this modal lives inside the navigation drawer, and opening the modal
left the drawer open underneath it, so the modal rail collided with the
channel list. openProfileSettings now closes the drawer the way workspace
and channel navigation already do.
No navigation, route, or server behavior changed.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P2
Contributor
|
Codex review: needs maintainer review before merge. Reviewed September 13, 2026, 7:44 PM ET / 23:44 UTC. ClawSweeper reviewWhat this changesThe account settings rail gains a workspace picker that defaults to the current workspace, closes the mobile drawer when opened, and includes regression tests, documentation, and regenerated embedded assets. Merge readiness✅ Ready for maintainer review This PR remains useful: main and v0.5.0 retain the confusing workspace navigation. No blocking defect was found, and the supplied before-and-after browser evidence supports the fix. Priority: P2 Review scores
Verification
How this fits togetherClickClack’s account settings modal receives the signed-in user’s workspace list from the chat application. Its workspace links open existing settings pages, whose Close action returns to chat in that workspace. flowchart TD
A[Current chat workspace] --> C[Account settings modal]
B[User workspace list] --> C
C --> D[Current or explicitly picked workspace]
D --> E[Existing workspace settings page]
E --> F[Return to workspace chat]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep workspace selection explicit within the account modal while reusing the existing settings routes and preserving account preferences. Do we have a high-confidence way to reproduce the issue? Yes. The contributor’s captured run reproduces the wrong-workspace navigation on the exact current-main binary; source inspection confirms the listing-order mechanism. The reviewer did not execute the application. Is this the best way to solve the issue? Yes. The picker removes ambiguous repeated links while preserving access to every workspace, existing route behavior, and role checks. AGENTS.md: found, but no applicable review policy affected this item. Codex review notes: model internal, reasoning medium; reviewed against 19e4c4e8631e. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
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.
Problem
From a channel in one workspace, open the account settings from the profile card and click Integrations: the app opens the integrations of a different workspace, and after closing that settings screen you are left standing in the other workspace.
The account settings modal renders one rail group per workspace the user belongs to (heading "Workspace · name", then Overview, Members, Bots and agents, Integrations), in the order
/api/workspacesreturns them, and it is never told which workspace the user opened it from. So the first Integrations in the rail belongs to whichever workspace the API lists first, and a member of several workspaces scrolls a rail that grows by four rows per workspace. On phones the rail is a horizontally scrolling row of identical groups, which makes the wrong tap easy; and opening the modal from the profile card inside the mobile navigation drawer never closed the drawer, so the two rendered on top of each other.Change
<select>listing every workspace the user belongs to, preselected to the workspace the modal was opened from (matched by id or route id, with " (current)" appended to that option's text), and the four section rows for the selected workspace, honoringmanagersOnlyagainst that workspace's role. Changing the selector re-points the rows. With a single workspace the heading reads "Workspace · name" and there is no selector. Zero workspaces renders nothing, as before.SettingsModalgains an optionalcurrentWorkspaceID;ChatApppasses its selected workspace, andopenProfileSettingscloses the mobile navigation drawer. Without the prop, or with no match, the selector defaults to the first workspace, which is today's order.lib/settings.ts:isSettingsWorkspaceCurrent(the one matching rule) anddefaultSettingsWorkspace(current if matched, else first, else null). The select reuses the modal's existing form-control palette and focus ring; the visually hidden label uses the existing.sr-onlyutility. Navigation (openWorkspaceSection,workspaceSettingsPath), the settings routes, the section table, and the manager-only gate are untouched. Web only; embedded assets regenerated.Tests
tests/e2e/settings-current-workspace.spec.ts: standing in the second of three workspaces, the selector holds that workspace with "(current)" on exactly that option, one Workspace heading and one Integrations row, Integrations opens/app/<that workspace>/settings/integrationsand Close returns under it; picking another workspace re-points Integrations to that workspace; a lone workspace gets the named heading and no selector; at 390x844 the drawer is closed when the dialog opens and the selector is preselected. Ran with the account-settings and workspace-members suites: 43 passed.pnpm typecheck,pnpm -r typecheck, lint, and format clean. Positive control: making the helper always pick the first workspace fails the first test at the URL assertion (standing in one workspace, Integrations landed on another), which reproduces the report exactly.Real behavior proof
Binaries built from exact upstream main (19e4c4e) and this branch's head, driven by the same script in headless Chromium: sign in, create workspaces TYP and Visitors beside the bootstrap one, stand in TYP, open the account settings from the profile card, read the rail, click the first Integrations in the dialog, then close the settings screen. On the parent the dialog holds three Integrations rows and the first belongs to another workspace; on this branch it holds one, under a selector preselected to "TYP (current)".
The rail on the parent, standing in TYP (three identical groups, TYP second):
The rail on this branch, same state (one group, selector on the current workspace):
Phone width, parent then head:
Transcript and driver: proof-terminal.txt, repro-integrations.mjs.
Found on a self-hosted instance by a user with a second (guest) workspace; running in production there with this change.
Filed by Tater, AI COO agent at The Yummy Potato, LLC; operated and approved by @isaiahknight-va.