feat(studio): apply MDCMS design system to Studio surfaces#141
Merged
Conversation
Reskins the Studio admin shell, dashboard, content list/overview, schema browser, and document editor against the MDCMS design system tokens (offwhite canvas, cobalt primary, lime accent, Space Grotesk display + Inter body + Geist Mono technical) introduced for the Studio runtime. - Sidebar: dark "ink" shell with the cube logomark, mono nav rows, admin section divider, and account footer; existing routes preserved. - Dashboard: 36px display heading, mono uppercase stat cards, narrower Content types column with i18n badge inline, Recently updated rail with one-line truncated mono meta, hover affordance with primary accent bar. - Content overview: per-type cards with letter mark, total/published/ drafts grid, percent-published bar, "browse →" footer. - Content list (per type): mono uppercase column headers, design-aligned PUBLISHED / DRAFT / UNPUBLISHED CHANGES status pills. - Schema page: split-pane master/detail with dashed registry strip (schemaHash, syncedAt, project, READ-ONLY IN STUDIO), kind chips, and resolvedSchema JSON tab. - Document editor: design-spec topbar with UNPUBLISHED CHANGES pill, Save draft (manual save bypasses auto-save debounce), and Publish; flat 30x30 mono toolbar with primary-tinted active state; canvas header with path chip and dashed frontmatter mono row above the centered editor body; "Type / to insert a component…" hint scoped via a new EmptyParagraphHint plugin to the focused empty top-level paragraph and rendered absolutely so it does not push the caret; redesigned slash menu with keyboard navigation (↑/↓/Enter/Tab/Esc). - Inspector: mono uppercase tab strip (Info / Properties / contextual Component / History) with primary bottom-border on the active tab. - Tokens: ports the design's sidebar, divider, blue-100, vibrant-green, code-bg, and shadow-pop tokens into the runtime stylesheet, switches the editor body to bg-background and the controls (topbar/toolbar/ inspector) to bg-card so the writing surface reads as the design's warm offwhite canvas while controls stay white. SPEC-006 is updated to specify the new editor topbar contract (Save draft, the UNPUBLISHED CHANGES badge, the trailing action cluster), the canvas header (path chip + frontmatter mono row), and the four-tab sidebar in normative terms.
📝 WalkthroughWalkthroughThis PR implements a comprehensive redesign of the Studio UI, focusing on improved editor affordances, refactored document workflows, and administrative interfaces. Changes include a new empty-paragraph hint extension for better component insertion, reorganized document editor pages with canvas headers and redesigned sidebars, a split-pane schema browser with field-level constraint chips, refactored dashboard with statistics cards, updated content list and overview pages with improved visual hierarchy, and an enhanced sidebar with session context integration. CSS theme tokens are extended for sidebar styling, and editor keyboard navigation is enhanced for the component picker. ChangesStudio UI Redesign
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/studio/src/lib/runtime-ui/app/admin/schema-page.tsx (1)
584-594:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the unused
contextprop fromSchemaPage.
SchemaPageis the only admin page that receives acontextparameter, yet it immediately silences the unused variable warning withvoid context;and reads all its data fromuseStudioMountInfo()instead. All other admin pages (UsersPage,SettingsPage,TrashPage, etc.) follow the pattern of not takingcontextat all. Remove the parameter from the component signature at line 585 and the registration call inremote-studio-app.tsxline 89 to align with the established pattern.🤖 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/studio/src/lib/runtime-ui/app/admin/schema-page.tsx` around lines 584 - 594, SchemaPage currently declares and silences an unused `context` prop; remove the prop from the component signature and its type annotation, delete the `void context;` line, and update any call sites that pass `context` (specifically the registration call that mounts or registers SchemaPage) to stop supplying that argument so SchemaPage matches other admin pages (e.g., UsersPage/SettingsPage patterns). Ensure the exported function is now `export default function SchemaPage() { ... }` and adjust the registration invocation to pass the component without a context parameter.packages/studio/src/lib/runtime-ui/components/editor/mdx-component-picker.tsx (1)
32-67:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
useEffectmust be moved above the early return — Rules of Hooks violation.
useRefis correctly placed at line 32 (before the early return), butuseEffectat line 59 is placed after the conditional early return at line 34. Hooks must always be used at the top level of a React function, before any early returns. Ifcomponentsever transitions from an empty array to a non-empty one (or vice versa) between renders — which is entirely possible if the MDX catalog loads asynchronously — React will throw"Rendered more/fewer hooks than during the previous render".Move the
useEffect(and keepuseRef) above the early return, putting the guard condition inside the hook body instead:🐛 Proposed fix
export function MdxComponentPicker({ ... }) { const listRef = useRef<HTMLDivElement | null>(null); + + // Scroll the highlighted row into view when the user arrows past the + // visible area of a tall component catalog. + useEffect(() => { + if (highlightedIndex === undefined) return; + const list = listRef.current; + if (!list) return; + const item = list.querySelector<HTMLElement>( + `[data-mdcms-mdx-picker-item-index="${highlightedIndex}"]`, + ); + item?.scrollIntoView({ block: "nearest" }); + }, [highlightedIndex]); if (components.length === 0) { return ( <section data-mdcms-mdx-picker="catalog" data-mdcms-mdx-picker-state="empty" className="..." > No local MDX components registered. </section> ); } const normalizedQuery = query.trim().toLowerCase(); // ...filtered items... - // Scroll the highlighted row into view when the user arrows past the - // visible area of a tall component catalog. - useEffect(() => { - if (highlightedIndex === undefined) return; - const list = listRef.current; - if (!list) return; - const item = list.querySelector<HTMLElement>( - `[data-mdcms-mdx-picker-item-index="${highlightedIndex}"]`, - ); - item?.scrollIntoView({ block: "nearest" }); - }, [highlightedIndex]);🤖 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/studio/src/lib/runtime-ui/components/editor/mdx-component-picker.tsx` around lines 32 - 67, The useEffect that scrolls the highlighted row is declared after the early return, causing a Rules of Hooks violation; move the useEffect block (which references listRef and highlightedIndex) above the early return that checks components.length === 0 so hooks are always called in the same order, and inside the effect add the existing guards (if highlightedIndex === undefined return; if !list return) before querying `[data-mdcms-mdx-picker-item-index="${highlightedIndex}"]` and calling scrollIntoView; keep useRef(listRef) where it is and leave the early return for rendering unchanged.
🧹 Nitpick comments (6)
packages/studio/src/lib/runtime-ui/pages/content-page.test.tsx (1)
136-141: ⚡ Quick winTighten subtitle assertion to validate computed summary values.
/content type/is too permissive; it won’t catch regressions in totals/localized count formatting. Assert the full expected subtitle for this fixture.Suggested fix
- assert.match(markup, /content type/); + assert.match(markup, /2 content types · 10 documents · 1 localized/);🤖 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/studio/src/lib/runtime-ui/pages/content-page.test.tsx` around lines 136 - 141, The assertion using assert.match(markup, /content type/) is too loose; update the test in content-page.test.tsx to assert the full computed subtitle string for the fixture instead of the generic regex—locate the assertions that use assert.match(markup, /content type/) (and nearby i18n/locale assertions) and replace the loose regex with an exact match of the expected subtitle text (using the same markup variable and assert method), ensuring totals and localized counts are validated precisely.packages/studio/src/lib/runtime-ui/app/admin/schema-page.tsx (2)
502-527: ⚡ Quick winTabs lack ARIA tab semantics.
The two
<button>triggers visually behave as a tablist but expose norole="tablist"/role="tab"/aria-selected/aria-controls, and the panel below has norole="tabpanel". Keyboard users and screen readers will hear two unrelated buttons and won't get arrow-key navigation. Adding the roles plusaria-selected={tab === "…"}is a small, contained improvement.🤖 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/studio/src/lib/runtime-ui/app/admin/schema-page.tsx` around lines 502 - 527, The two buttons are acting as tabs but are missing ARIA roles and attributes; wrap the button group div with role="tablist", give each button role="tab" plus aria-selected={tab === "fields" || tab === "source"} and aria-controls pointing to the corresponding panel id, and add matching id attributes on the panel(s) with role="tabpanel" (e.g., id="fields-panel" and id="source-panel"); keep using the existing tab state and setTab handler to toggle aria-selected and the visible panel, and ensure focus/keyboard handling uses the tab roles so arrow-key navigation works with the native button handlers or by adding simple keydown handling if needed.
117-126: 💤 Low valueReplace hardcoded
rgba()values with semantic tokens where they correspond.The kind palette mixes design-system tokens (
bg-vibrant-green,bg-code-bg) with hardcodedrgba(47,73,229,0.10)andtext-[#516600]that bypass the token layer. However, note that not all hardcoded rgba values map exactly to defined tokens—onlyrgba(47,73,229,0.10)(line 118) andrgba(47,73,229,0.12)(line 169) match--primaryat 10% and 12% opacity, which could becomebg-primary/10andbg-primary/12in Tailwind v4. Thetext-[#516600]should be migrated to a semantic foreground token. Other rgba values may be intentional custom overrides; verify against the design system before replacing.Minor:
activeTypestate can become desynchronized whenentrieschanges. WhileuseMemoensures the renderedentrystays correct, the state variable lags behind. Consider resetting or reconcilingactiveTypewhenentriesidentity changes.Minor: Tab buttons lack ARIA semantics. Lines 503–527 use
<button>withoutrole="tab",aria-selected, ortabpanelassociation. Consider adding these attributes for screen reader clarity.Remove the unused
contextprop at line 594. The component receivescontextbut ignores it, deriving all state fromuseStudioMountInfo(). The prop is passed by the caller atremote-studio-app.tsx; update both the component signature and the call site to remove it.🤖 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/studio/src/lib/runtime-ui/app/admin/schema-page.tsx` around lines 117 - 126, KIND_CHIP_CLASSES contains hardcoded rgba and hex colors—replace rgba(47,73,229,0.10) and rgba(47,73,229,0.12) with semantic Tailwind tokens (e.g. bg-primary/10 and bg-primary/12) and swap text-[`#516600`] for the corresponding semantic foreground token; leave intentional custom rgba values alone after verifying with design. Also reconcile the activeType state with changes to entries by resetting or syncing activeType whenever entries identity updates (references: activeType, entries, and the useMemo'd entry). Add proper ARIA semantics to the tab buttons (give each button role="tab", aria-selected, and associate with a tabpanel) so screen readers can navigate the tabs. Finally remove the unused context prop from the component signature and its caller (the component derives state from useStudioMountInfo()), updating both the component and remote-studio-app.tsx call site.packages/studio/src/lib/runtime-ui/components/layout/app-sidebar.tsx (2)
279-290: ⚡ Quick win
bg-accent-subtleas a test oracle class is fragile — use adata-*attribute instead.The comment acknowledges this is an "inert marker class" added only so that
app-sidebar.test.tsxcan assert active routing. This is problematic for two reasons:
Potential visual regression: In Tailwind v4, the cascade order between
bg-sidebar-accent(set inbaseClasses) andbg-accent-subtle(appended after) is determined by the CSS output order, not the class attribute order. If Tailwind emitsbg-accent-subtleafterbg-sidebar-accent, the active link background is silently overridden to the subtle accent color instead of the intended sidebar accent.Coupling concerns: Styling classes carry semantic weight; using one purely as a test hook will confuse future readers and can break when the design token is renamed.
Prefer a semantically neutral
data-activeattribute:♻️ Proposed fix
- // Append `bg-accent-subtle` as an inert marker class on the active link. - // It's a no-op visually (the active background is bg-sidebar-accent above) - // but is asserted by app-sidebar.test.tsx to verify active routing under - // scenario base paths in the studio-review host. - const className = isActive ? `${baseClasses} bg-accent-subtle` : baseClasses; + const className = baseClasses; const link = ( - <Link href={resolvedHref} className={className}> + <Link href={resolvedHref} className={className} data-active={isActive ? "true" : undefined}>Then update
app-sidebar.test.tsxto assertdata-active="true"instead of the class.🤖 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/studio/src/lib/runtime-ui/components/layout/app-sidebar.tsx` around lines 279 - 290, Replace the inert test-only marker class usage: instead of appending "bg-accent-subtle" to className when isActive, set a semantic data attribute (e.g. data-active="true") on the element; update the code paths that compute className (referencing baseClasses and className) to not rely on the marker class and add the data-active attribute when isActive is true, and update app-sidebar.test.tsx to assert data-active="true" (and remove/adjust the comment about the inert marker class).
136-145: ⚡ Quick win
accountMetaternary has dead logic —sessioncondition never affects the output.Both truthy branches of the ternary evaluate to
mountInfo.project, making thesessioncheck in the first branch a no-op. The whole expression collapses tomountInfo.project ?? "MDCMS". If the intent was to include the user's identifier when a session is present (e.g., display the project + email domain), the value computation needs to be updated to actually usesession.♻️ Proposed simplification (if richer session metadata is not intended)
- const accountMeta = - mountInfo.project && session - ? `${mountInfo.project}` - : mountInfo.project - ? mountInfo.project - : "MDCMS"; + const accountMeta = mountInfo.project ?? "MDCMS";🤖 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/studio/src/lib/runtime-ui/components/layout/app-sidebar.tsx` around lines 136 - 145, The accountMeta ternary currently checks session but both branches return mountInfo.project, so replace the expression to either (a) simplify to use mountInfo.project ?? "MDCMS" if you don't intend to include session info, or (b) if you do want session-specific metadata, construct a value that actually uses session (for example `${mountInfo.project} • ${session.email}` or `${mountInfo.project} — ${deriveDisplayName(session.email)}`) — update the accountMeta assignment accordingly (referencing the accountMeta and session variables and mountInfo.project).packages/studio/src/lib/runtime-ui/pages/content-document-page.tsx (1)
2058-2080: ⚡ Quick winAdd tab semantics to the new sidebar tabstrip.
These controls now behave like tabs, but assistive tech only gets a row of generic buttons. Adding
role="tablist",role="tab",aria-selected, and panel associations would make the active pane discoverable without changing the visual design.Suggested fix
-function SidebarTabButton({ +function SidebarTabButton({ + id, + controls, label, active, onClick, }: { + id: string; + controls: string; label: string; active: boolean; onClick: () => void; }) { return ( <button type="button" + id={id} + role="tab" + aria-selected={active} + aria-controls={controls} onClick={onClick} className={cn( "flex-1 border-b-2 border-transparent py-2.5 text-center font-mono text-[11px] uppercase tracking-wider transition-colors", active ? "border-primary text-foreground" : "text-foreground-muted hover:text-foreground", )} > {label} </button> ); }-<div className="flex border-b border-border"> +<div role="tablist" aria-label="Document sidebar sections" className="flex border-b border-border">Also applies to: 2519-2542
🤖 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/studio/src/lib/runtime-ui/pages/content-document-page.tsx` around lines 2058 - 2080, Wrap the tab buttons container with role="tablist" and update each SidebarTabButton to include role="tab", aria-selected={active}, aria-controls linking to the corresponding panel id, and set tabIndex={active ? 0 : -1} so only the active tab is in sequential focus; ensure the panel elements use matching ids and include role="tabpanel" and aria-labelledby pointing back to the tab id. Locate the SidebarTabButton component and the parent tabstrip render (and the corresponding panel render around the pane content) to add the attributes and id conventions (e.g., generate stable ids from the label or a provided key) so assistive tech can discover the active pane without changing visuals.
🤖 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.
Inline comments:
In `@packages/studio/src/lib/runtime-ui/components/editor/tiptap-editor.tsx`:
- Around line 751-790: The Enter/Tab handlers are swallowing keystrokes even
when there is nothing to insert; update the handlers so they only consume the
key when there is an actionable item and the picker is actually visible.
Concretely, in the block gated by pickerSourceRef.current === "slash" add a
guard around the Enter and Tab handling that checks
filteredSlashComponentsRef.current.length > 0 and that the picker is visible
(e.g. verify slashPickerCoordsRef.current !== null or that slashTrigger/current
state exists) before reading slashHighlightIndexRef.current and calling
insertSelectedComponentRef.current; if the guard fails, do not return true so
the editor receives the key event. Also apply the same visibility+non-empty
guard to the Enter/Tab cases (ArrowUp/ArrowDown already check items.length).
In `@packages/studio/src/lib/runtime-ui/pages/content-document-page.tsx`:
- Around line 2746-2758: The Save Draft button currently allows manual saves
while a historical version is open (state.viewingVersion), causing hidden
drafts; update the Button disabled logic and the shared save routine to prevent
any saves when state.viewingVersion is truthy: in the UI component (where the
Button uses state.saveState, state.publishState and onSaveNow) add a guard that
disables the button when state.viewingVersion is set, and then add the same
early-return guard inside the central save handler(s) (the onSaveNow entry-point
and the debounced autosave handler/shared save routine) so both manual and
autosave paths no-op when viewingVersion is active.
In `@packages/studio/src/lib/runtime-ui/pages/content-page.tsx`:
- Around line 44-45: Compute and clamp the published percentage to the 0–100
range immediately after it's derived so both the text and the progress bar use
the same safe value: replace the current publishedPercentage calculation (which
uses publishedCount and totalCount) with a clamped value (e.g., calculate
rawPercent = totalCount > 0 ? Math.round((publishedCount / totalCount) * 100) :
0, then publishedPercentage = Math.max(0, Math.min(100, rawPercent))). Update
any uses (the variable referenced where the text is rendered and the bar width
code around the block at lines ~107-112) to consume this single clamped
publishedPercentage. Ensure publishedCount/totalCount edge cases (published >
total or negative counts) are handled by the clamp.
---
Outside diff comments:
In `@packages/studio/src/lib/runtime-ui/app/admin/schema-page.tsx`:
- Around line 584-594: SchemaPage currently declares and silences an unused
`context` prop; remove the prop from the component signature and its type
annotation, delete the `void context;` line, and update any call sites that pass
`context` (specifically the registration call that mounts or registers
SchemaPage) to stop supplying that argument so SchemaPage matches other admin
pages (e.g., UsersPage/SettingsPage patterns). Ensure the exported function is
now `export default function SchemaPage() { ... }` and adjust the registration
invocation to pass the component without a context parameter.
In
`@packages/studio/src/lib/runtime-ui/components/editor/mdx-component-picker.tsx`:
- Around line 32-67: The useEffect that scrolls the highlighted row is declared
after the early return, causing a Rules of Hooks violation; move the useEffect
block (which references listRef and highlightedIndex) above the early return
that checks components.length === 0 so hooks are always called in the same
order, and inside the effect add the existing guards (if highlightedIndex ===
undefined return; if !list return) before querying
`[data-mdcms-mdx-picker-item-index="${highlightedIndex}"]` and calling
scrollIntoView; keep useRef(listRef) where it is and leave the early return for
rendering unchanged.
---
Nitpick comments:
In `@packages/studio/src/lib/runtime-ui/app/admin/schema-page.tsx`:
- Around line 502-527: The two buttons are acting as tabs but are missing ARIA
roles and attributes; wrap the button group div with role="tablist", give each
button role="tab" plus aria-selected={tab === "fields" || tab === "source"} and
aria-controls pointing to the corresponding panel id, and add matching id
attributes on the panel(s) with role="tabpanel" (e.g., id="fields-panel" and
id="source-panel"); keep using the existing tab state and setTab handler to
toggle aria-selected and the visible panel, and ensure focus/keyboard handling
uses the tab roles so arrow-key navigation works with the native button handlers
or by adding simple keydown handling if needed.
- Around line 117-126: KIND_CHIP_CLASSES contains hardcoded rgba and hex
colors—replace rgba(47,73,229,0.10) and rgba(47,73,229,0.12) with semantic
Tailwind tokens (e.g. bg-primary/10 and bg-primary/12) and swap text-[`#516600`]
for the corresponding semantic foreground token; leave intentional custom rgba
values alone after verifying with design. Also reconcile the activeType state
with changes to entries by resetting or syncing activeType whenever entries
identity updates (references: activeType, entries, and the useMemo'd entry). Add
proper ARIA semantics to the tab buttons (give each button role="tab",
aria-selected, and associate with a tabpanel) so screen readers can navigate the
tabs. Finally remove the unused context prop from the component signature and
its caller (the component derives state from useStudioMountInfo()), updating
both the component and remote-studio-app.tsx call site.
In `@packages/studio/src/lib/runtime-ui/components/layout/app-sidebar.tsx`:
- Around line 279-290: Replace the inert test-only marker class usage: instead
of appending "bg-accent-subtle" to className when isActive, set a semantic data
attribute (e.g. data-active="true") on the element; update the code paths that
compute className (referencing baseClasses and className) to not rely on the
marker class and add the data-active attribute when isActive is true, and update
app-sidebar.test.tsx to assert data-active="true" (and remove/adjust the comment
about the inert marker class).
- Around line 136-145: The accountMeta ternary currently checks session but both
branches return mountInfo.project, so replace the expression to either (a)
simplify to use mountInfo.project ?? "MDCMS" if you don't intend to include
session info, or (b) if you do want session-specific metadata, construct a value
that actually uses session (for example `${mountInfo.project} •
${session.email}` or `${mountInfo.project} —
${deriveDisplayName(session.email)}`) — update the accountMeta assignment
accordingly (referencing the accountMeta and session variables and
mountInfo.project).
In `@packages/studio/src/lib/runtime-ui/pages/content-document-page.tsx`:
- Around line 2058-2080: Wrap the tab buttons container with role="tablist" and
update each SidebarTabButton to include role="tab", aria-selected={active},
aria-controls linking to the corresponding panel id, and set tabIndex={active ?
0 : -1} so only the active tab is in sequential focus; ensure the panel elements
use matching ids and include role="tabpanel" and aria-labelledby pointing back
to the tab id. Locate the SidebarTabButton component and the parent tabstrip
render (and the corresponding panel render around the pane content) to add the
attributes and id conventions (e.g., generate stable ids from the label or a
provided key) so assistive tech can discover the active pane without changing
visuals.
In `@packages/studio/src/lib/runtime-ui/pages/content-page.test.tsx`:
- Around line 136-141: The assertion using assert.match(markup, /content type/)
is too loose; update the test in content-page.test.tsx to assert the full
computed subtitle string for the fixture instead of the generic regex—locate the
assertions that use assert.match(markup, /content type/) (and nearby i18n/locale
assertions) and replace the loose regex with an exact match of the expected
subtitle text (using the same markup variable and assert method), ensuring
totals and localized counts are validated precisely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 141ae5eb-fe56-4340-9d9b-37268887bfce
📒 Files selected for processing (18)
.changeset/studio-redesign-empty-paragraph-hint.mdapps/studio-example/lib/studio-example-studio-config.tsdocs/specs/SPEC-006-studio-runtime-and-ui.mdpackages/studio/src/lib/editor-extensions.tspackages/studio/src/lib/runtime-ui/app/admin/content/[type]/page.tsxpackages/studio/src/lib/runtime-ui/app/admin/page.tsxpackages/studio/src/lib/runtime-ui/app/admin/schema-page.test.tsxpackages/studio/src/lib/runtime-ui/app/admin/schema-page.tsxpackages/studio/src/lib/runtime-ui/components/editor/mdx-component-picker.test.tsxpackages/studio/src/lib/runtime-ui/components/editor/mdx-component-picker.tsxpackages/studio/src/lib/runtime-ui/components/editor/tiptap-editor.tsxpackages/studio/src/lib/runtime-ui/components/layout/app-sidebar.tsxpackages/studio/src/lib/runtime-ui/components/mdcms-logo.tsxpackages/studio/src/lib/runtime-ui/pages/content-document-page.test.tsxpackages/studio/src/lib/runtime-ui/pages/content-document-page.tsxpackages/studio/src/lib/runtime-ui/pages/content-page.test.tsxpackages/studio/src/lib/runtime-ui/pages/content-page.tsxpackages/studio/src/lib/runtime-ui/styles.css
- Editor slash picker: gate Enter/Tab on filteredSlashComponentsRef having items AND the picker actually being visible (slashTrigger + slashPickerCoords refs). When no item is actionable, return false so the editor receives the keystroke. ArrowUp/ArrowDown likewise no-op on an empty filtered list. - Document editor: refuse to persist drafts while a historical version is being viewed. The Save draft button is disabled when state.viewingVersion is truthy, and both the autosave debounce and the central saveDraft routine early-return on the same condition so the manual button and the timer share one guard. "Restore this version" remains the only path to materialize a historical body as a new draft. - Content overview cards: clamp the published percentage to [0, 100] immediately after deriving it so the "% published" label and the progress bar both consume the same safe value when upstream returns published > total or a negative count. - SchemaPage: drop the unused `context` prop (`void context;` was the only use). The component already reads everything it needs from useStudioMountInfo(); update remote-studio-app.tsx to render <SchemaPage /> without an argument so it matches the Users/Settings/Promote pattern. - MdxComponentPicker: move the scroll-into-view useEffect above the empty-catalog early return so hooks always fire in the same order (Rules of Hooks). - AppSidebar: replace the inert `bg-accent-subtle` marker class on the active nav row with a semantic `data-active="true"` attribute and update app-sidebar.test.tsx to anchor on the attribute. The accountMeta ternary that returned mountInfo.project either way is collapsed to `mountInfo.project ?? "MDCMS"`. - content-page test: replace the loose `/content type/` regex with the exact computed subtitle for the fixture. Drop the changeset added in the previous commit — every code path edited here lives under packages/studio/src/lib/runtime-ui/, which is listed in the package's .changeset-gate.json `unpublishedSources` and is served by the backend runtime, not exported through @mdcms/studio's npm surface. Skipped (nitpick / out of scope for this fix-up): - ARIA tablist/tab/tabpanel roles on the Schema and editor sidebar tabs (a focused a11y pass; visuals unchanged either way). - KIND_CHIP_CLASSES rgba/hex literals in schema-page (values are the exact design-system primary/olive tints).
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/studio/src/lib/runtime-ui/components/editor/mdx-component-picker.tsx (1)
120-125: ⚡ Quick winReplace
bg-blue-100with a semantic token to preserve dark-mode and theming correctness.Every other color in this file uses semantic design-system tokens (
bg-card,bg-accent-subtle,text-foreground,text-primary,border-border, etc.) that automatically adapt to theme changes and dark mode.bg-blue-100is a hardcoded palette value that will render as a fixed light blue regardless of theme, which is inconsistent with the system and will break in a dark-mode context.Using a single semantic token rather than two atomic tokens means the component adapts automatically — instead of needing both
bg-blue-800anddark:bg-blue-600, a single semantic alias handles both modes transparently.Replace with either the existing
bg-primary/10tint (if the design token is defined that way) or a dedicatedbg-accent/bg-primary-subtlesemantic token consistent with the project's@themedefinitions.🎨 Proposed fix
- isHighlighted - ? "bg-blue-100 text-foreground" - : "text-foreground hover:bg-accent-subtle", + isHighlighted + ? "bg-primary/10 text-foreground" + : "text-foreground hover:bg-accent-subtle",🤖 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/studio/src/lib/runtime-ui/components/editor/mdx-component-picker.tsx` around lines 120 - 125, The highlighted line uses a hardcoded color token ("bg-blue-100") inside the className built with cn for the MDX component picker; replace that hardcoded palette class with the project's semantic design token (e.g., "bg-primary/10" or a semantic alias like "bg-accent" or "bg-primary-subtle") so the highlighted state adapts to dark mode and theming. Update the conditional branch where isHighlighted is used in the className expression in mdx-component-picker.tsx (the cn(...) call) to use the chosen semantic token instead of "bg-blue-100" while preserving the existing "text-foreground" text class.packages/studio/src/lib/runtime-ui/pages/content-document-page.tsx (1)
2001-2056: ⚡ Quick win
DocumentCanvasHeader— redundant cast and nullable-locale edge caseTwo minor observations:
- Line 2012:
(fm as Record<string, unknown>)[key]—fmis alreadyRecord<string, unknown>, so the cast is a no-op and can be removed.- Line 2033:
fmEntries.push(["locale", state.locale])always appends the locale, even whenstate.localeis the internal placeholder"__mdcms_default__"(used when a document has no real locale, see line 1340 for the inference path). This would display the raw placeholder string to the user in the canvas header.✨ Proposed fix
- const value = (fm as Record<string, unknown>)[key]; + const value = fm[key];- fmEntries.push(["locale", state.locale]); + if (state.locale && state.locale !== "__mdcms_default__") { + fmEntries.push(["locale", state.locale]); + }🤖 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/studio/src/lib/runtime-ui/pages/content-document-page.tsx` around lines 2001 - 2056, Remove the unnecessary cast in pickValue by accessing fm directly (replace (fm as Record<string, unknown>)[key] with fm[key]) and prevent showing the internal placeholder locale by only pushing ["locale", state.locale] into fmEntries when state.locale is defined and not equal to "__mdcms_default__"; update DocumentCanvasHeader (pickValue, fmEntries population) accordingly so the cast is removed and the placeholder locale is skipped.
🤖 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.
Inline comments:
In `@packages/studio/src/lib/runtime-ui/pages/content-document-page.tsx`:
- Around line 2058-2081: The SidebarTabButton component and its surrounding
markup lack ARIA tab semantics; update SidebarTabButton to render the button
with role="tab", set aria-selected={active} and provide an id and aria-controls
that reference the matching panel id, ensure the container component
(ContentDocumentPageSidebar) that renders the tab buttons has role="tablist",
and update the corresponding panel render to use role="tabpanel" with
aria-labelledby pointing back to the tab id (and include proper id
generation/matching between SidebarTabButton and the panel). Ensure the active
tab has tabIndex=0 and inactive tabs have tabIndex=-1 so keyboard navigation
works.
---
Nitpick comments:
In
`@packages/studio/src/lib/runtime-ui/components/editor/mdx-component-picker.tsx`:
- Around line 120-125: The highlighted line uses a hardcoded color token
("bg-blue-100") inside the className built with cn for the MDX component picker;
replace that hardcoded palette class with the project's semantic design token
(e.g., "bg-primary/10" or a semantic alias like "bg-accent" or
"bg-primary-subtle") so the highlighted state adapts to dark mode and theming.
Update the conditional branch where isHighlighted is used in the className
expression in mdx-component-picker.tsx (the cn(...) call) to use the chosen
semantic token instead of "bg-blue-100" while preserving the existing
"text-foreground" text class.
In `@packages/studio/src/lib/runtime-ui/pages/content-document-page.tsx`:
- Around line 2001-2056: Remove the unnecessary cast in pickValue by accessing
fm directly (replace (fm as Record<string, unknown>)[key] with fm[key]) and
prevent showing the internal placeholder locale by only pushing ["locale",
state.locale] into fmEntries when state.locale is defined and not equal to
"__mdcms_default__"; update DocumentCanvasHeader (pickValue, fmEntries
population) accordingly so the cast is removed and the placeholder locale is
skipped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: df9a309d-24fb-406d-ab9c-0367107db080
📒 Files selected for processing (9)
packages/studio/src/lib/remote-studio-app.tsxpackages/studio/src/lib/runtime-ui/app/admin/schema-page.tsxpackages/studio/src/lib/runtime-ui/components/editor/mdx-component-picker.tsxpackages/studio/src/lib/runtime-ui/components/editor/tiptap-editor.tsxpackages/studio/src/lib/runtime-ui/components/layout/app-sidebar.test.tsxpackages/studio/src/lib/runtime-ui/components/layout/app-sidebar.tsxpackages/studio/src/lib/runtime-ui/pages/content-document-page.tsxpackages/studio/src/lib/runtime-ui/pages/content-page.test.tsxpackages/studio/src/lib/runtime-ui/pages/content-page.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/studio/src/lib/runtime-ui/pages/content-page.tsx
- packages/studio/src/lib/runtime-ui/components/layout/app-sidebar.tsx
- packages/studio/src/lib/runtime-ui/pages/content-page.test.tsx
- packages/studio/src/lib/runtime-ui/components/editor/tiptap-editor.tsx
- packages/studio/src/lib/runtime-ui/app/admin/schema-page.tsx
Comment on lines
+2058
to
+2081
| function SidebarTabButton({ | ||
| label, | ||
| active, | ||
| onClick, | ||
| }: { | ||
| label: string; | ||
| active: boolean; | ||
| onClick: () => void; | ||
| }) { | ||
| return ( | ||
| <button | ||
| type="button" | ||
| onClick={onClick} | ||
| className={cn( | ||
| "flex-1 border-b-2 border-transparent py-2.5 text-center font-mono text-[11px] uppercase tracking-wider transition-colors", | ||
| active | ||
| ? "border-primary text-foreground" | ||
| : "text-foreground-muted hover:text-foreground", | ||
| )} | ||
| > | ||
| {label} | ||
| </button> | ||
| ); | ||
| } |
There was a problem hiding this comment.
SidebarTabButton is missing ARIA tab semantics — screen readers cannot identify the tab pattern
<button type="button"> without role="tab" and aria-selected does not announce "tab, selected/not selected" to assistive technology. The container <div> wrapping the tab buttons also needs role="tablist", and the rendered panel needs role="tabpanel" with a matching aria-labelledby.
♿ Proposed fix
function SidebarTabButton({
label,
active,
onClick,
+ id,
+ panelId,
}: {
label: string;
active: boolean;
onClick: () => void;
+ id: string;
+ panelId: string;
}) {
return (
<button
type="button"
+ role="tab"
+ id={id}
+ aria-selected={active}
+ aria-controls={panelId}
+ tabIndex={active ? 0 : -1}
onClick={onClick}
className={cn(
"flex-1 border-b-2 border-transparent py-2.5 text-center font-mono text-[11px] uppercase tracking-wider transition-colors",
active
? "border-primary text-foreground"
: "text-foreground-muted hover:text-foreground",
)}
>
{label}
</button>
);
}And in ContentDocumentPageSidebar:
- <div className="flex border-b border-border">
+ <div role="tablist" aria-label="Document sidebar" className="flex border-b border-border">
<SidebarTabButton
label="Info"
active={activeTab === "info"}
onClick={() => setActiveTab("info")}
+ id="sidebar-tab-info"
+ panelId="sidebar-panel"
/>
...
</div>
- <div className="flex-1 overflow-y-auto">
+ <div role="tabpanel" id="sidebar-panel" aria-labelledby={`sidebar-tab-${activeTab}`} className="flex-1 overflow-y-auto">Also applies to: 2511-2568
🤖 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/studio/src/lib/runtime-ui/pages/content-document-page.tsx` around
lines 2058 - 2081, The SidebarTabButton component and its surrounding markup
lack ARIA tab semantics; update SidebarTabButton to render the button with
role="tab", set aria-selected={active} and provide an id and aria-controls that
reference the matching panel id, ensure the container component
(ContentDocumentPageSidebar) that renders the tab buttons has role="tablist",
and update the corresponding panel render to use role="tabpanel" with
aria-labelledby pointing back to the tab id (and include proper id
generation/matching between SidebarTabButton and the panel). Ensure the active
tab has tabIndex=0 and inactive tabs have tabIndex=-1 so keyboard navigation
works.
This was referenced May 15, 2026
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.
Summary
EmptyParagraphHintProseMirror plugin so the design's "Type / to insert a component…" affordance only fires on the focused empty top-level paragraph and is rendered absolutely so the caret stays at column zero.↑/↓/Enter/Tab/Esc) + mouse-hover sync to the slash component picker.Save draftbutton that bypasses the 5 s auto-save debounce by calling the same persistence routine; the UNPUBLISHED CHANGES badge replaces the redundantChangedworkflow pill andv{version}chip.docs/specs/SPEC-006-studio-runtime-and-ui.mdto specify the new editor topbar contract (UNPUBLISHED CHANGES badge, Save draft, trailing action cluster), the canvas header (path chip + dashed frontmatter mono row), and the four-tab sidebar (Info / Properties / contextual Component / History) in normative terms.Visual highlights
read-onlyhint on Schema, admin section divider, account footer.i18nbadge + percent-published bar, wide Recently updated rail with truncated mono meta and hover affordance.browse →footer.PUBLISHED,DRAFT,UNPUBLISHED CHANGES).schemaHash,syncedAt,project,READ-ONLY IN STUDIO), kind chips,resolvedSchema (JSON)tab.Color tokens
bg-background(off-white in light, ink in dark) so the writing surface reads as canvas.bg-card(white in light, card-grey in dark) as design controls.border-bordertoken consistently.Test plan
bun run typecheck(studio only — workspace nx is out of sync from upstreamcore.system/domain.contentmodules unrelated to this change)bun test --cwd packages/studio ./src— 588 pass / 1 unrelated pre-existing failure/adminand verify the dashboard layout, hover affordances, and stat-card spacing/admin/content— overview cards render with i18n badge, percent bar, andbrowse →link/admin/content/:type— table headers are mono uppercase; status pills renderPUBLISHED/DRAFT/UNPUBLISHED CHANGES/admin/schema— split-pane shows dashed registry strip + per-type kind chips + JSON tab/admin/content/:type/:documentId— UNPUBLISHED CHANGES pill appears only when the doc has draft changes;Save draftghost button persists immediately and disables when saved; toolbar pinned at top while body scrolls; canvas centered to ~880 px; path chip + dashed frontmatter row align with body width/→↑/↓walks the picker,Enterinserts,Tabinserts,Esccloses; mouse hover and keyboard cursor stay in syncInfo / Properties / History(Component appears only when an MDX node is selected); default tab is PropertiesSummary by CodeRabbit
Release Notes
New Features
Style