Skip to content

Commit 9ed23b4

Browse files
authored
Merge pull request #1066 from org2AI/dev/consolidate-layout-settings
refactor(layout-settings): consolidate station layout controls
2 parents c6752fa + 72ce443 commit 9ed23b4

51 files changed

Lines changed: 438 additions & 412 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# Chat panel position unification architecture audit
2+
3+
## Acceptance criteria
4+
5+
- One canonical persisted setting controls the chat-panel side in every station layout
6+
- One writable atom owns reads and writes for that setting
7+
- Settings and Spotlight expose one choice rather than station-specific choices
8+
- Existing split settings migrate deterministically without changing the default for new users
9+
- No production reference remains to the old atoms, action IDs, or station-specific labels
10+
- Obsolete station-specific and superseded menu translation keys are removed from every locale
11+
12+
## Ten-layer audit
13+
14+
| Layer | Verdict | Evidence |
15+
| ----------------------------------------- | ------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
16+
| 1. Compilation correctness | pass | TypeScript typecheck and focused tests cover the schema, atom, Settings menu, Spotlight definition, and action registration. |
17+
| 2. Dead code and structural deduplication | pass | Two schema fields, two atoms, four actions, two Spotlight choices, and the station-selection branches were replaced by one source and one action pair. The repository sweep found no old runtime identifier; eight obsolete keys were removed from all 13 locales. |
18+
| 3. Naming consistency | pass | `general.chatPanelPosition` and `chatPanelPositionAtom` describe the setting without tying it to either station. Legacy names occur only in the migration boundary and its tests. |
19+
| 4. Semantic overloading | pass | “Station” now describes the active layout only; it no longer selects a separate meaning for chat-panel position. |
20+
| 5. Default branch analysis | pass | New users retain the left-side default. Migration precedence is explicit: canonical value, valid legacy My Station value, valid legacy Agent Station value, then the schema default. |
21+
| 6. Cross-domain leakage | pass | Shared layout consumers import a station-neutral atom; no My Station or Agent Station position concept leaks into shared state. |
22+
| 7. New-developer clarity | pass | One schema key, atom, UI row, and action pair form a direct ownership chain. |
23+
| 8. Wire and serialization | pass / scoped | No network or IPC payload changed. The settings JSONC persistence surface changes to one canonical key; old keys are accepted only during validation and removed from normalized runtime state. |
24+
| 9. Init parity | pass | Startup hydration and external settings-file changes both call `validateSettings`, so both entry paths apply the same migration and canonical projection. |
25+
| 10. Resolver symmetry | pass | Both station layouts resolve the same field through the same atom. There is no longer a station-dependent fallback branch. |
26+
27+
## Ownership path
28+
29+
| Entry point | Canonical write/read | My Station consumer | Agent Station consumer |
30+
| ----------------------- | ------------------------------------------------ | -------------------------------------------- | ---------------------------------------------- |
31+
| Layout settings submenu | `chatPanelPositionAtom` | App layout and workstation header affordance | App layout and agent-station header affordance |
32+
| Action system | `chatPanelPositionAtom` | Same shared projection | Same shared projection |
33+
| Spotlight | Shared left/right action IDs | Same shared projection | Same shared projection |
34+
| Settings hydration | `validateSettings``general.chatPanelPosition` | Same shared projection | Same shared projection |
35+
36+
## Migration invariant
37+
38+
`src/config/settingsSchema/index.ts:55` is the only compatibility boundary. A valid canonical value always wins. If it is absent, the prior My Station value wins when valid because it was the primary workstation preference; a valid prior Agent Station value is the fallback. The normalized settings object drops both legacy keys, and startup backfills the canonical key to disk.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# Sidebar Layout settings menu UI audit
2+
3+
| Line | Element | Verdict | Reason | Suggested change |
4+
| ---------------------------------------------------------------------------- | ------------------------------- | ---------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- |
5+
| `src/scaffold/NavigationSidebar/blocks/SidebarSettingsMenuButton.tsx:511` | Layout submenu trigger | fix | The separate Chat Panel and Workstation triggers duplicated one layout-settings surface. One localized Layout row now owns the complete submenu. | None. |
6+
| `src/scaffold/NavigationSidebar/blocks/SidebarSettingsMenuSubmenus.tsx:84` | Layout submenu routing | fix | A single `layout` state replaces the two presentation-only modes, so menu state matches the consolidated information architecture. | None. |
7+
| `src/components/SegmentedTextPill/index.tsx:9` | Segmented-control size API | fix | A typed `small` / `default` prop lets compact menus reduce the control without changing established creator controls. The default preserves the prior dimensions. | None. |
8+
| `src/components/SegmentedTextPill/index.tsx:21` | Segmented-control size geometry | keep with reason | The 24px small control fits the shared 32px menu row, while the original 28px control remains the established Agent/Manual geometry. Both variants stay centralized in the design-system component. | None. |
9+
| `src/scaffold/NavigationSidebar/blocks/SidebarLayoutSettingsSubmenu.tsx:37` | Compact segmented setting row | abstract | Three choice-based settings share one local row seam instead of rebuilding title/control alignment. | Keep new choice-based Layout settings on `SegmentedControlRow`. |
10+
| `src/scaffold/NavigationSidebar/blocks/SidebarLayoutSettingsSubmenu.tsx:49` | Dropdown setting-row chrome | keep with reason | `DROPDOWN_CLASSES.menuControlItem` supplies the established menu height, spacing, typography, hover treatment, and label/control alignment. | None. |
11+
| `src/scaffold/NavigationSidebar/blocks/SidebarLayoutSettingsSubmenu.tsx:51` | Left/right and style selectors | keep with reason | Reuses the shared `SegmentedTextPill` that owns the Agent/Manual visual pattern requested for this menu; each group exposes an accessible name and pressed state. | None. |
12+
| `src/scaffold/NavigationSidebar/blocks/SidebarLayoutSettingsSubmenu.tsx:74` | Pagination control | fix | The shared Switch remains the correct boolean control and receives the visible row label as its accessible name. | None. |
13+
| `src/scaffold/NavigationSidebar/blocks/SidebarLayoutSettingsSubmenu.tsx:127` | Shared chat-panel side row | fix | One station-independent Left/Right control replaces separate My Station and Agent Station rows, preventing contradictory choices in the UI. | None. |
14+
| `src/scaffold/NavigationSidebar/blocks/SidebarLayoutSettingsSubmenu.tsx:121` | Floating submenu surface | keep with reason | Reuses the dropdown panel and width tokens shared by the sibling Presence and Appearance submenus. | None. |
15+
16+
Verdict totals: **5 fix**, **4 keep with reason**, **1 abstract**.

src/ActionSystem/actionIds.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,8 @@ export const ACTION_ID = {
3939
WORKSTATION_TOGGLE_CHAT_FOCUS: "workstation.toggleChatFocus",
4040
WORKSTATION_TOGGLE_CHAT_PANEL_VISIBILITY:
4141
"workstation.toggleChatPanelVisibility",
42-
CHAT_PANEL_SET_MY_STATION_LEFT: "chatPanel.setMyStationLeft",
43-
CHAT_PANEL_SET_MY_STATION_RIGHT: "chatPanel.setMyStationRight",
44-
CHAT_PANEL_SET_AGENT_STATION_LEFT: "chatPanel.setAgentStationLeft",
45-
CHAT_PANEL_SET_AGENT_STATION_RIGHT: "chatPanel.setAgentStationRight",
42+
CHAT_PANEL_SET_LEFT: "chatPanel.setLeft",
43+
CHAT_PANEL_SET_RIGHT: "chatPanel.setRight",
4644
CHAT_PANEL_ENABLE_PAGINATION: "chatPanel.enablePagination",
4745
CHAT_PANEL_DISABLE_PAGINATION: "chatPanel.disablePagination",
4846
CHAT_PANEL_USE_MODEL_PICKER_SPOTLIGHT: "chatPanel.useModelPickerSpotlight",

src/ActionSystem/actions/chatPanelActions.zod.ts

Lines changed: 17 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ import {
1010
} from "@src/store/ui/chatPanelAtom";
1111
import { stationModeAtom } from "@src/store/ui/simulatorAtom";
1212
import {
13-
sessionChatPositionAtom,
14-
workStationChatPositionAtom,
13+
chatPanelPositionAtom,
1514
workStationLayoutModePersistAtom,
1615
} from "@src/store/ui/workStationAtom";
1716
import { getInstrumentedStore } from "@src/util/core/state/instrumentedStore";
@@ -53,56 +52,33 @@ function defineEmptyAction(
5352
);
5453
}
5554

56-
function setMyStationChatPosition(position: ChatPanelPosition): void {
55+
function setChatPanelPosition(position: ChatPanelPosition): void {
5756
const store = getInstrumentedStore();
5857
showActiveStationChatIfNeeded();
59-
store.set(workStationChatPositionAtom, position);
60-
}
61-
62-
function setAgentStationChatPosition(position: ChatPanelPosition): void {
63-
const store = getInstrumentedStore();
64-
store.set(sessionChatPositionAtom, position);
58+
store.set(chatPanelPositionAtom, position);
6559
}
6660

6761
function setModelPickerStyle(style: ModelPickerStyle): void {
6862
const store = getInstrumentedStore();
6963
store.set(modelPickerStyleAtom, style);
7064
}
7165

72-
const chatPanelSetMyStationLeft = defineEmptyAction(
73-
ACTION_ID.CHAT_PANEL_SET_MY_STATION_LEFT,
74-
"settings",
75-
"Move the My Station chat panel to the left",
76-
"My Station chat panel moved left",
77-
["move my station chat left", "put chat panel on the left"],
78-
() => setMyStationChatPosition("left")
79-
);
80-
81-
const chatPanelSetMyStationRight = defineEmptyAction(
82-
ACTION_ID.CHAT_PANEL_SET_MY_STATION_RIGHT,
83-
"settings",
84-
"Move the My Station chat panel to the right",
85-
"My Station chat panel moved right",
86-
["move my station chat right", "put chat panel on the right"],
87-
() => setMyStationChatPosition("right")
88-
);
89-
90-
const chatPanelSetAgentStationLeft = defineEmptyAction(
91-
ACTION_ID.CHAT_PANEL_SET_AGENT_STATION_LEFT,
66+
const chatPanelSetLeft = defineEmptyAction(
67+
ACTION_ID.CHAT_PANEL_SET_LEFT,
9268
"settings",
93-
"Move the Agent Station chat panel to the left",
94-
"Agent Station chat panel moved left",
95-
["move agent station chat left", "put agent chat on the left"],
96-
() => setAgentStationChatPosition("left")
69+
"Move the chat panel to the left in every station layout",
70+
"Chat panel moved left",
71+
["move chat panel left", "put chat panel on the left"],
72+
() => setChatPanelPosition("left")
9773
);
9874

99-
const chatPanelSetAgentStationRight = defineEmptyAction(
100-
ACTION_ID.CHAT_PANEL_SET_AGENT_STATION_RIGHT,
75+
const chatPanelSetRight = defineEmptyAction(
76+
ACTION_ID.CHAT_PANEL_SET_RIGHT,
10177
"settings",
102-
"Move the Agent Station chat panel to the right",
103-
"Agent Station chat panel moved right",
104-
["move agent station chat right", "put agent chat on the right"],
105-
() => setAgentStationChatPosition("right")
78+
"Move the chat panel to the right in every station layout",
79+
"Chat panel moved right",
80+
["move chat panel right", "put chat panel on the right"],
81+
() => setChatPanelPosition("right")
10682
);
10783

10884
const chatPanelEnablePagination = defineEmptyAction(
@@ -172,10 +148,8 @@ const workstationSetSidebarRight = defineEmptyAction(
172148
);
173149

174150
export const chatPanelZodActions = [
175-
chatPanelSetMyStationLeft,
176-
chatPanelSetMyStationRight,
177-
chatPanelSetAgentStationLeft,
178-
chatPanelSetAgentStationRight,
151+
chatPanelSetLeft,
152+
chatPanelSetRight,
179153
chatPanelEnablePagination,
180154
chatPanelDisablePagination,
181155
chatPanelUseModelPickerSpotlight,
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { createElement } from "react";
2+
import { renderToStaticMarkup } from "react-dom/server";
3+
import { describe, expect, it } from "vitest";
4+
5+
import SegmentedTextPill from ".";
6+
7+
const options = [
8+
{ value: "left", label: "Left" },
9+
{ value: "right", label: "Right" },
10+
];
11+
12+
function renderPill(size?: "small" | "default"): string {
13+
return renderToStaticMarkup(
14+
createElement(SegmentedTextPill, {
15+
ariaLabel: "Position",
16+
onChange: () => undefined,
17+
options,
18+
size,
19+
value: "left",
20+
})
21+
);
22+
}
23+
24+
describe("SegmentedTextPill", () => {
25+
it("preserves the established dimensions by default", () => {
26+
const markup = renderPill();
27+
28+
expect(markup).toContain("h-[28px]");
29+
expect(markup).toContain("h-6 px-2.5");
30+
});
31+
32+
it("offers a smaller variant for compact setting rows", () => {
33+
const markup = renderPill("small");
34+
35+
expect(markup).toContain("h-6 text-[11px]");
36+
expect(markup).toContain("h-5 px-2");
37+
expect(markup).toContain('aria-label="Position"');
38+
});
39+
});

src/components/SegmentedTextPill/index.tsx

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,42 @@ export interface SegmentedTextPillOption<T extends string> {
66
value: T;
77
}
88

9+
export type SegmentedTextPillSize = "small" | "default";
10+
911
export interface SegmentedTextPillProps<T extends string> {
1012
ariaLabel: string;
1113
className?: string;
1214
dataTestId?: string;
1315
onChange: (value: T) => void;
1416
options: SegmentedTextPillOption<T>[];
17+
size?: SegmentedTextPillSize;
1518
value: T;
1619
}
1720

21+
const CONTAINER_SIZE_CLASSES: Record<SegmentedTextPillSize, string> = {
22+
small: "h-6 text-[11px]",
23+
default: "h-[28px] text-[12px]",
24+
};
25+
26+
const BUTTON_SIZE_CLASSES: Record<SegmentedTextPillSize, string> = {
27+
small: "h-5 px-2",
28+
default: "h-6 px-2.5",
29+
};
30+
1831
/** Compact text-only segmented control shared by creator setup rows. */
1932
export default function SegmentedTextPill<T extends string>({
2033
ariaLabel,
2134
className = "",
2235
dataTestId,
2336
onChange,
2437
options,
38+
size = "default",
2539
value,
2640
}: SegmentedTextPillProps<T>) {
2741
return (
2842
<div
2943
aria-label={ariaLabel}
30-
className={`inline-flex h-[28px] shrink-0 items-center rounded-full bg-fill-2 p-0.5 text-[12px] font-medium ${className}`}
44+
className={`inline-flex shrink-0 items-center rounded-full bg-fill-2 p-0.5 font-medium ${CONTAINER_SIZE_CLASSES[size]} ${className}`}
3145
data-testid={dataTestId}
3246
role="group"
3347
>
@@ -38,7 +52,7 @@ export default function SegmentedTextPill<T extends string>({
3852
<button
3953
key={option.value}
4054
type="button"
41-
className={`h-6 rounded-full px-2.5 py-0 transition-colors ${
55+
className={`rounded-full py-0 transition-colors ${BUTTON_SIZE_CLASSES[size]} ${
4256
selected
4357
? "bg-bg-2 text-text-1 shadow-sm"
4458
: "text-text-3 hover:text-text-1"
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import {
2+
getSettingsDefaults,
3+
validateSettings,
4+
} from "@src/config/settingsSchema";
5+
6+
describe("chat panel position setting", () => {
7+
it("has one canonical setting shared by both station layouts", () => {
8+
const defaults = getSettingsDefaults();
9+
10+
expect(defaults["general.chatPanelPosition"]).toBe("left");
11+
expect(defaults).not.toHaveProperty("general.workStationChatPosition");
12+
expect(defaults).not.toHaveProperty("general.sessionChatPosition");
13+
});
14+
15+
it("preserves an explicit canonical position", () => {
16+
expect(
17+
validateSettings({
18+
"general.chatPanelPosition": "right",
19+
"general.workStationChatPosition": "left",
20+
"general.sessionChatPosition": "left",
21+
})["general.chatPanelPosition"]
22+
).toBe("right");
23+
});
24+
25+
it("migrates My Station first when legacy positions disagree", () => {
26+
expect(
27+
validateSettings({
28+
"general.workStationChatPosition": "right",
29+
"general.sessionChatPosition": "left",
30+
})["general.chatPanelPosition"]
31+
).toBe("right");
32+
});
33+
34+
it("falls back to the legacy Agent Station position when needed", () => {
35+
expect(
36+
validateSettings({
37+
"general.workStationChatPosition": "invalid",
38+
"general.sessionChatPosition": "right",
39+
})["general.chatPanelPosition"]
40+
).toBe("right");
41+
});
42+
});

0 commit comments

Comments
 (0)