Repository navigation
feat: D3 global styles UI (typography, colors, layout, blocks, elements, variations) - #376
Merged
Merged
Conversation
…ements, variations)
Mounts the global styles inspector inside the D1 shell so authors can
customize theme.json-shaped presets and styles from the site editor,
diffed live against the base payload and written back through C3's
`PUT /global-styles/{id}`.
Closes #370
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a complete D3 Global Styles feature: a new Styles section (navigator, canvas, inspector), editor hooks and API client, immutable theme.json utilities, multiple style panels (typography, colors, layout, blocks, elements, variations), UI/stylesheet assets, and extensive Vitest coverage for load, interactions, apply, save, and validation flows. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Browser Client
participant Shell as D1 Shell
participant StylesSection as useStylesSectionViews
participant Editor as useGlobalStylesEditor
participant API as Global Styles API
participant Store as Core Data Store
Client->>Shell: Mount Styles Section (enabled=true)
Shell->>StylesSection: call useStylesSectionViews
StylesSection->>Editor: initialize editor (enabled)
Editor->>API: lookup() → id
API-->>Editor: { id }
Editor->>API: fetch base
API-->>Editor: GlobalStylesBase
Editor->>Store: dispatch base & id
Editor->>API: fetch current global-styles (id)
API-->>Editor: GlobalStylesRecord
Editor->>Editor: compute merged draft
Editor-->>StylesSection: return navigator, canvas, inspector elements + state
Client->>Editor: setValue / applyVariation / save
Editor->>API: PUT updateGlobalStyles(id, payload)
API-->>Editor: 200 OK or 422 Validation Error
Editor-->>StylesSection: propagate saveStatus / validationErrors / isDirty
StylesSection-->>Shell: shell onStateChange (entity-like state)
sequenceDiagram
participant Browser as Client
participant Canvas as StyleBookCanvas
participant Inspector as StylesInspector
participant Editor as useGlobalStylesEditor
participant API as Global Styles API
Browser->>Canvas: Click variation button (slug)
Canvas->>Inspector: onSelectVariation(slug)
Inspector->>Editor: applyVariation(slug)
Editor->>Editor: replace draft.settings/styles with variation
Editor-->>Inspector: updated draft, isDirty=true
Browser->>Inspector: Click Save
Inspector->>Editor: save()
Editor->>API: PUT /global-styles/{id}
alt 200 OK
API-->>Editor: 200
Editor-->>Inspector: saveStatus='saved', isDirty=false
else 422 Validation Error
API-->>Editor: 422 + body
Editor-->>Inspector: validationErrors, saveStatus='error'
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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)
Comment |
Capture the delegate fetcher before overriding global.fetch so the outer afterEach restore path points at the correct stub — the previous ordering captured the override itself, making "restore" a no-op. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (2)
resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsx (2)
84-97: Case-insensitive comparison may miss format variations.The
toLowerCase()comparison handles hex case differences (#FFFvs#fff), but won't match equivalent colors in different formats (e.g.,#fffvs#fffffforrgb(255,255,255)).If palette colors are always stored in a consistent format (likely), this is fine. Otherwise, consider using a color normalization utility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsx` around lines 84 - 97, paletteRefFromColor currently compares colors with simple toLowerCase checks which misses equivalent colors in different formats; update the function to normalize both the input color and palette entry colors to a canonical format before comparing (e.g., expand shorthand hex to full hex, convert rgb/rgba/hsl to hex or a normalized lowercase hex) using a shared normalizeColor utility or an existing color library, then compare normalized values and return the same `var(--wp--preset--color--${match.slug})` when matched; ensure the normalization is applied to both the `color` parameter and each `entry.color` in the `palette`.
272-284: Consider exhaustive kind handling for future maintainability.The default branch handles both
'size'and any unrecognizedStyleFieldKindvalues. If a new kind is added to the union type, it would silently fall through toUnitControl.An explicit
'size'branch with adefault: neverguard would catch missing handlers at compile time.🔧 Optional: Exhaustive type checking
+ if (descriptor.kind === 'size') { + return ( + <StyleControlRow key={descriptor.testId} {...rowProps}> + <UnitControl + label={__(descriptor.label, TEXT_DOMAIN)} + value={value} + units={SIZE_UNITS} + data-testid={`ap-site-editor-style-field-input-${descriptor.testId}`} + __nextHasNoMarginBottom={true} + onChange={(next) => editor.setValue(path, next ?? '')} + /> + </StyleControlRow> + ); + } + + // Exhaustive check - TypeScript will error if a kind is unhandled + const _exhaustive: never = descriptor.kind; + throw new Error(`Unhandled field kind: ${_exhaustive}`); - return ( - <StyleControlRow key={descriptor.testId} {...rowProps}> - <UnitControl - label={__(descriptor.label, TEXT_DOMAIN)} - value={value} - units={SIZE_UNITS} - data-testid={`ap-site-editor-style-field-input-${descriptor.testId}`} - __nextHasNoMarginBottom={true} - onChange={(next) => editor.setValue(path, next ?? '')} - /> - </StyleControlRow> - );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsx` around lines 272 - 284, The switch currently treats 'size' and any unknown StyleFieldKind values the same by rendering UnitControl inside StyleControlRow; update the handler for the union of StyleFieldKind to include an explicit 'size' case that returns the UnitControl JSX (using descriptor.testId, UnitControl, StyleControlRow and editor.setValue) and add a default branch that narrows to never (e.g., `const _exhaustive: never = kind;`) so the compiler will error if a new StyleFieldKind is added without a corresponding case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/js/visual-editor/site-editor/styles/panels/colors-panel.tsx`:
- Around line 252-256: paletteError only reads
validationErrors['settings.color.palette'] so per-entry keys like
'settings.color.palette.1.slug' are ignored; update the validation handling to
collect and surface per-entry errors by scanning validationErrors keys that
start with 'settings.color.palette.' and mapping them to the corresponding
palette index/property, then pass those row-specific messages into the
swatch/row renderer instead of (or in addition to) the single paletteError
variable; look for usages of paletteError, validationErrors, and rendering of
palette rows/swatches in this file (e.g., the paletteError declaration and the
component that renders each palette entry) and wire the per-entry messages into
the component props so each swatch can show inline feedback.
- Around line 74-94: The slug stored for swatches must be normalized the same
way created slugs are: use the existing slugify(value, existing) function to
transform any manual edit before persisting so stored slugs are safe for
constructing CSS custom property names (var(--wp--preset--color--${slug})), and
similarly run slugify where slugs are set/updated (references: slugify,
resolveValueForPalette) including the manual-edit handlers around the other
occurrences noted; ensure you pass the current list of existing slugs as the
second arg so collisions get the -N suffix.
In `@resources/js/visual-editor/site-editor/styles/panels/panel-controls.css`:
- Around line 77-87: Remove the deprecated word-break: break-word declarations
from the .ap-site-editor__style-control-row-base rule and its child
.ap-site-editor__style-control-row-base > code rule; keep the existing
overflow-wrap: anywhere property which already provides the needed wrapping
behavior and is sufficient for modern browsers, so delete the two word-break
lines associated with those selectors.
- Around line 126-130: The separator rule under
.ap-site-editor__styles-inspector .components-panel__row +
.components-panel__row uses the camelCase keyword currentColor; change it to
lowercase currentcolor so it conforms to the stylelint value-keyword-case:
"lower" rule and avoids the lint error in the color-mix(...) expression.
In `@resources/js/visual-editor/site-editor/styles/panels/typography-panel.tsx`:
- Around line 70-86: The select's "Custom…" option never actually enters custom
mode because the onChange branch writes back the existing preset value, so
fontFamilyValue still matches presets and the select snaps back; update the
onChange handler(s) for FONT_FAMILY_PATH (and analogues for font size) so when
value === CUSTOM_SENTINEL you clear or set the draft value to an empty/explicit
custom marker (e.g., '' or null) instead of re-writing the preset value,
ensuring fontFamilyMatches (computed from presets.fontFamilies and
fontFamilyValue) becomes false and the custom input can render; reference
symbols: FONT_FAMILY_PATH, fontFamilyValue, fontFamilySelectValue,
CUSTOM_SENTINEL, presets.fontFamilies and the onChange that currently writes the
draft.
- Around line 107-123: The current reset handlers call editor.resetPath(), which
only clears pending edits and can re-expose an existing persisted override;
change the handlers to call a "reset to theme default" operation instead:
replace onResetSection's WATCHED_PATHS.forEach((path) => editor.resetPath(path))
with WATCHED_PATHS.forEach((path) => editor.resetPathToDefault(path)) and
replace individual onReset={() => editor.resetPath(FONT_FAMILY_PATH)} with
onReset={() => editor.resetPathToDefault(FONT_FAMILY_PATH)} (and similarly for
other paths). If editor.resetPathToDefault does not exist, implement
editor.resetPathToDefault(path) so it both clears the pending edit and
removes/overrides the persisted global-styles override for that path (using the
theme default lookup for that path) so the UI shows the true theme default in
baseValue.
In `@resources/js/visual-editor/site-editor/styles/style-book-canvas.css`:
- Around line 21-35: Update the color-mix() calls inside the
.ap-site-editor__style-book-variation and
.ap-site-editor__style-book-variation[data-active='true'] rules to use the
lowercase keyword currentcolor instead of currentColor; locate every occurrence
of color-mix(in srgb, currentColor ...) (including the borders and background
usages and the other spots noted around lines 56–57 and 86) and replace
currentColor with currentcolor so the rules comply with value-keyword-case:
"lower".
In `@resources/js/visual-editor/site-editor/styles/style-book-canvas.tsx`:
- Around line 160-205: The radiogroup implements roving-tabindex but lacks
keyboard handlers: add an onKeyDown handler (e.g., handleVariationKeyDown) on
the container (the element with role="radiogroup") that listens for
ArrowLeft/ArrowUp and ArrowRight/ArrowDown, computes the next/previous variation
index (wrapping at ends), calls onSelectVariation with the target
variation.slug, and moves focus to the corresponding button; to reliably focus
the button elements, maintain an array of refs (one per variation) or query by
the existing data-testid
(`ap-site-editor-style-book-variation-${variation.slug}`) and call .focus() on
the target; ensure the handler also prevents default for arrow keys and
preserves the existing roving-tabindex logic (activeVariationSlug and index
checks).
In `@resources/js/visual-editor/site-editor/styles/styles-inspector.css`:
- Around line 168-180: Remove the deprecated declarations of "word-break:
break-word" from the CSS rules for .ap-site-editor__style-listing-label and
.ap-site-editor__style-listing-code; keep the existing overflow-wrap: anywhere
and any other properties intact so behavior remains the same and stylelint
errors are resolved.
In `@resources/js/visual-editor/site-editor/styles/styles-navigator.css`:
- Around line 42-48: The CSS uses the color keyword "currentColor" with
uppercase C inside color-mix(), triggering stylelint; update all occurrences to
the lowercase "currentcolor" (e.g., in the selectors
.ap-site-editor__styles-navigator-link:hover and
.ap-site-editor__styles-navigator-item[data-active='true'] >
.ap-site-editor__styles-navigator-link and the other three color-mix usages
referenced in the comment) so every color-mix(in srgb, currentColor X%,
transparent) becomes color-mix(in srgb, currentcolor X%, transparent).
In `@resources/js/visual-editor/site-editor/styles/styles-navigator.tsx`:
- Around line 85-118: The UI declares a tree widget but provides no
keyboard/tree interaction; remove the ARIA tree semantics instead of
implementing a full tree: change the <nav>/<ul> from role="tree" to
role="navigation" (or remove role) and the <ul> to role="list" and each <li>
from role="treeitem" to role="listitem", remove aria-expanded on <li> (since
expand/collapse is not keyboard-managed) and remove aria-selected in favor of
keeping aria-current on the <button> (use topLevelActive(state, panel) and
state.panel to set aria-current), and retain PANEL_ORDER, getPanelLabel and
existing data-* attributes so visual behavior stays the same.
In `@resources/js/visual-editor/site-editor/styles/styles-section.tsx`:
- Around line 140-147: selectActiveVariation currently returns activeSlug
unconditionally which leaves a stale active variation after manual edits; update
selectActiveVariation(editor: UseGlobalStylesEditorResult, variations: readonly
StyleVariation[], activeSlug: string | null) to validate that activeSlug both
exists in variations (find by slug) AND that the editor's current draft matches
that variation's definition (compare the draft/style fields the variation
represents) before returning it, otherwise return null; apply the same
validation logic to the other places noted (the blocks referenced around the
activeVariationSlug logic at the other ranges) so any code that currently trusts
activeVariationSlug first checks existence in variations and equality with the
current draft.
In `@resources/js/visual-editor/site-editor/styles/use-global-styles-editor.ts`:
- Around line 153-180: buildDraft currently merges base, record, then edits so
keys present in record survive resetPath/replaceSubtree; to fix, change the
merge logic so edits can remove or fully replace record values: treat
replaceSubtree at a given path as ignoring record for that subtree (i.e., build
subtree from base then edits only), and have resetPath produce explicit delete
markers (or track removed paths) that are applied when merging record (so
mergeDeep skips/omits those keys from record). Update buildDraft to call a new
merge routine that accepts a set of removed paths and obeys replaceSubtree
semantics, and adjust usages of mergeDeep, resetPath, and replaceSubtree to
operate with these removal/replace rules so saved record keys do not survive a
reset or root-level replace.
- Around line 252-255: When enabled flips to false, invalidate any in-flight
generation and clear the editor state: increment requestCounterRef.current (to
abort stale responses), reset id/base/record via
setId(null)/setBase(null)/setRecord(initial or null), clear save/load errors via
setSaveError(null)/setLoadError(null) and then setLoadStatus('idle'); place this
logic inside the existing useEffect branch that checks !enabled so pending async
callbacks check the bumped requestCounterRef and cannot commit stale styles.
In `@resources/js/visual-editor/site-editor/styles/use-registered-blocks.ts`:
- Around line 100-103: When the useEffect early-returns because enabled is false
it doesn’t advance requestCounterRef or reset state, so inflight fetches can
still update blocks/error and leave isLoading true; inside the useEffect branch
for !enabled, increment requestCounterRef.current and call the state resetters
(setBlocks([] or null as used), setError(null), setIsLoading(false)) to
invalidate in-flight responses and clear the hook state so subsequent resolved
promises check the bumped requestCounterRef and no longer overwrite state.
---
Nitpick comments:
In `@resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsx`:
- Around line 84-97: paletteRefFromColor currently compares colors with simple
toLowerCase checks which misses equivalent colors in different formats; update
the function to normalize both the input color and palette entry colors to a
canonical format before comparing (e.g., expand shorthand hex to full hex,
convert rgb/rgba/hsl to hex or a normalized lowercase hex) using a shared
normalizeColor utility or an existing color library, then compare normalized
values and return the same `var(--wp--preset--color--${match.slug})` when
matched; ensure the normalization is applied to both the `color` parameter and
each `entry.color` in the `palette`.
- Around line 272-284: The switch currently treats 'size' and any unknown
StyleFieldKind values the same by rendering UnitControl inside StyleControlRow;
update the handler for the union of StyleFieldKind to include an explicit 'size'
case that returns the UnitControl JSX (using descriptor.testId, UnitControl,
StyleControlRow and editor.setValue) and add a default branch that narrows to
never (e.g., `const _exhaustive: never = kind;`) so the compiler will error if a
new StyleFieldKind is added without a corresponding case.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 550edfa8-e445-4bbe-992e-585c9abd2e6d
📒 Files selected for processing (26)
resources/js/visual-editor/site-editor/__tests__/site-editor-app.test.tsxresources/js/visual-editor/site-editor/site-editor-app.tsxresources/js/visual-editor/site-editor/styles/__tests__/styles-section.test.tsxresources/js/visual-editor/site-editor/styles/__tests__/theme-json-paths.test.tsresources/js/visual-editor/site-editor/styles/global-styles-api.tsresources/js/visual-editor/site-editor/styles/panels/blocks-panel.tsxresources/js/visual-editor/site-editor/styles/panels/colors-panel.tsxresources/js/visual-editor/site-editor/styles/panels/elements-panel.tsxresources/js/visual-editor/site-editor/styles/panels/layout-panel.tsxresources/js/visual-editor/site-editor/styles/panels/panel-controls.cssresources/js/visual-editor/site-editor/styles/panels/panel-controls.tsxresources/js/visual-editor/site-editor/styles/panels/styles-fields.tsxresources/js/visual-editor/site-editor/styles/panels/typography-panel.tsxresources/js/visual-editor/site-editor/styles/panels/use-preset-data.tsresources/js/visual-editor/site-editor/styles/panels/variations-panel.tsxresources/js/visual-editor/site-editor/styles/style-book-canvas.cssresources/js/visual-editor/site-editor/styles/style-book-canvas.tsxresources/js/visual-editor/site-editor/styles/styles-inspector.cssresources/js/visual-editor/site-editor/styles/styles-inspector.tsxresources/js/visual-editor/site-editor/styles/styles-navigator-tree.tsresources/js/visual-editor/site-editor/styles/styles-navigator.cssresources/js/visual-editor/site-editor/styles/styles-navigator.tsxresources/js/visual-editor/site-editor/styles/styles-section.tsxresources/js/visual-editor/site-editor/styles/theme-json-paths.tsresources/js/visual-editor/site-editor/styles/use-global-styles-editor.tsresources/js/visual-editor/site-editor/styles/use-registered-blocks.ts
Main corrections:
- Reset now genuinely returns to theme default: the edits state tracks
removed paths, stripped from the record during buildDraft and again
from the save payload so a previously-persisted override doesn't
survive.
- Picking "Custom…" in a preset dropdown actually enters custom mode —
previously we re-wrote the existing preset value, which snapped the
select back and hid the custom input.
- Variation picker exposes arrow-key navigation with wrap-around and
stops advertising a preset as "applied" after the user manually
edits anything that makes the draft diverge from the variation's
payload.
- Palette slug edits are normalized to theme.json-safe characters on
every keystroke, and per-row validation errors from C3's
`settings.color.palette.{index}.{slug|name|color}` response keys now
surface on the offending row.
- `useGlobalStylesEditor` and `useRegisteredBlocks` clear cached state
+ bump their request counters when `enabled` flips to false so a
stale in-flight response can't hydrate over the reset.
- Navigator swaps the non-keyboard-managed tree semantics for plain
`role="list"` + `aria-current="page"` on each link.
- CSS lint: `currentColor` → `currentcolor` in every `color-mix()` call;
redundant `word-break: break-word` declarations dropped where
`overflow-wrap: anywhere` already covers it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
resources/js/visual-editor/site-editor/styles/use-global-styles-editor.ts (2)
340-355:⚠️ Potential issue | 🟡 MinorClear
lastSavedAtwhen the editor is disabled.This branch now clears most cached state, but it still preserves the previous session’s save timestamp. Re-enabling the section can therefore report a stale “last saved” time before the new session has saved anything.
Suggested fix
if (!enabled) { // Invalidate any in-flight fetches started while this // section was active and clear the cached state so a // subsequent re-enable doesn't surface stale styles. requestCounterRef.current += 1; setId(null); setBase(null); setRecord(null); setEdits({ ...EMPTY_EDITS }); setLoadStatus('idle'); setLoadErrorMessage(null); setSaveStatus('idle'); setSaveErrorMessage(null); setValidationErrors(null); + setLastSavedAt(null); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/styles/use-global-styles-editor.ts` around lines 340 - 355, The disable branch inside the useEffect currently resets many cached states but leaves lastSavedAt intact; update this block to also clear the save timestamp by calling the state setter that manages lastSavedAt (e.g., setLastSavedAt(null)) so the editor doesn't show a stale “last saved” time when re-enabled; locate the disable branch in the useEffect and add the setLastSavedAt(null) call alongside setId(null), setBase(null), setRecord(null), etc.
507-537:⚠️ Potential issue | 🔴 Critical
replaceSubtree()still overlays persisted data instead of replacing it.
replaceSubtree()only writes intoedits, butbuildDraft()andbuildSavePayload()still merge the stripped saved record underneath that overlay. Any keys omitted fromvaluetherefore survive the “replace”, so applying a variation or replacing a nested subtree keeps stale saved overrides instead of overwriting them.This breaks the stated overwrite semantics for variations and any subtree-level wholesale replace. You need an explicit “replaced subtree” marker so merge logic can ignore
recordbeneath that path rather than layering it back in.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/styles/use-global-styles-editor.ts` around lines 507 - 537, replaceSubtree currently only writes into edits so buildDraft() / buildSavePayload() still merge saved record beneath an intended replace; add an explicit "replaced subtree" marker to the editor state and populate it from replaceSubtree (e.g. add a replacedPaths or replacedSubtrees Set/array on the same state object and in replaceSubtree: when path.length===0 mark the whole scope replaced, otherwise add that ThemeJsonPath to replacedPaths and still update removedPaths as needed). Then update buildDraft() and buildSavePayload() to consult replacedPaths/replacedSubtrees and, for any marked path (or whole-scope marker), ignore/skip merging the underlying record data under that path so the edits overlay becomes authoritative (i.e., treat marked paths as if record is absent beneath them). Ensure functions and state keys referenced are replaceSubtree, buildDraft, buildSavePayload, edits, removedPaths, and the new replacedPaths/replacedSubtrees marker.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsx`:
- Around line 47-53: The FONT_WEIGHT_PRESETS array currently hardcodes English
labels; update the array in styles/panels/styles-fields.tsx so each label is
wrapped with the localization function (e.g., use __ from '@wordpress/i18n' or
wp.i18n.__) instead of plain strings, e.g. replace 'Light · 300' with __('Light
· 300', 'your-text-domain') for every entry in FONT_WEIGHT_PRESETS and ensure __
is imported at the top of the file.
---
Duplicate comments:
In `@resources/js/visual-editor/site-editor/styles/use-global-styles-editor.ts`:
- Around line 340-355: The disable branch inside the useEffect currently resets
many cached states but leaves lastSavedAt intact; update this block to also
clear the save timestamp by calling the state setter that manages lastSavedAt
(e.g., setLastSavedAt(null)) so the editor doesn't show a stale “last saved”
time when re-enabled; locate the disable branch in the useEffect and add the
setLastSavedAt(null) call alongside setId(null), setBase(null), setRecord(null),
etc.
- Around line 507-537: replaceSubtree currently only writes into edits so
buildDraft() / buildSavePayload() still merge saved record beneath an intended
replace; add an explicit "replaced subtree" marker to the editor state and
populate it from replaceSubtree (e.g. add a replacedPaths or replacedSubtrees
Set/array on the same state object and in replaceSubtree: when path.length===0
mark the whole scope replaced, otherwise add that ThemeJsonPath to replacedPaths
and still update removedPaths as needed). Then update buildDraft() and
buildSavePayload() to consult replacedPaths/replacedSubtrees and, for any marked
path (or whole-scope marker), ignore/skip merging the underlying record data
under that path so the edits overlay becomes authoritative (i.e., treat marked
paths as if record is absent beneath them). Ensure functions and state keys
referenced are replaceSubtree, buildDraft, buildSavePayload, edits,
removedPaths, and the new replacedPaths/replacedSubtrees marker.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 788408fd-a659-46fd-9612-9b4b15c8ad57
📒 Files selected for processing (12)
resources/js/visual-editor/site-editor/styles/panels/colors-panel.tsxresources/js/visual-editor/site-editor/styles/panels/panel-controls.cssresources/js/visual-editor/site-editor/styles/panels/styles-fields.tsxresources/js/visual-editor/site-editor/styles/panels/typography-panel.tsxresources/js/visual-editor/site-editor/styles/style-book-canvas.cssresources/js/visual-editor/site-editor/styles/style-book-canvas.tsxresources/js/visual-editor/site-editor/styles/styles-inspector.cssresources/js/visual-editor/site-editor/styles/styles-navigator.cssresources/js/visual-editor/site-editor/styles/styles-navigator.tsxresources/js/visual-editor/site-editor/styles/styles-section.tsxresources/js/visual-editor/site-editor/styles/use-global-styles-editor.tsresources/js/visual-editor/site-editor/styles/use-registered-blocks.ts
✅ Files skipped from review due to trivial changes (4)
- resources/js/visual-editor/site-editor/styles/styles-navigator.css
- resources/js/visual-editor/site-editor/styles/styles-inspector.css
- resources/js/visual-editor/site-editor/styles/style-book-canvas.css
- resources/js/visual-editor/site-editor/styles/panels/panel-controls.css
🚧 Files skipped from review as they are similar to previous changes (4)
- resources/js/visual-editor/site-editor/styles/style-book-canvas.tsx
- resources/js/visual-editor/site-editor/styles/panels/typography-panel.tsx
- resources/js/visual-editor/site-editor/styles/panels/colors-panel.tsx
- resources/js/visual-editor/site-editor/styles/use-registered-blocks.ts
The array sat at module-level so every label was frozen as a raw English string before `bootI18n()` could swap in a translated text domain. Split into a value-only constant (for match detection) + a getter that wraps each label in `__()` at render time, mirroring the pattern `getSiteEditorSections` uses in `sections.tsx`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsx`:
- Line 158: Update the label handling so translation extraction works: wrap the
hardcoded descriptor label values with __() where they are declared (update the
label fields in the descriptor objects inside blocks-panel.tsx and
elements-panel.tsx to use __('... ', TEXT_DOMAIN) for each label), and remove
the runtime wrapping in styles-fields.tsx so the render uses descriptor.label
directly (replace occurrences of __('descriptor.label', ...) /
__(descriptor.label, TEXT_DOMAIN) in styles-fields.tsx with just
descriptor.label). Target the descriptor objects named "label" in
blocks-panel.tsx and elements-panel.tsx and the render usage of descriptor.label
in styles-fields.tsx.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c2e5f8ad-0775-4a7d-a245-6c8e98d7eef3
📒 Files selected for processing (1)
resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsx
| data-testid={`ap-site-editor-style-swatches-${descriptor.testId}`} | ||
| > | ||
| <span className="ap-site-editor__style-color-field-label"> | ||
| {__(descriptor.label, TEXT_DOMAIN)} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify descriptor labels are localized at declaration sites with literal __() calls.
# Expected: descriptor objects in blocks/elements panels use label: __('...', TEXT_DOMAIN)
rg -n -C2 "label:\s*__\(" resources/js/visual-editor/site-editor/styles/panels/blocks-panel.tsx resources/js/visual-editor/site-editor/styles/panels/elements-panel.tsx
rg -n -C2 "label:\s*['\"]" resources/js/visual-editor/site-editor/styles/panels/blocks-panel.tsx resources/js/visual-editor/site-editor/styles/panels/elements-panel.tsxRepository: ArtisanPack-UI/visual-editor
Length of output: 5089
🏁 Script executed:
# Check complete structure of descriptor objects in declaration files
head -100 resources/js/visual-editor/site-editor/styles/panels/blocks-panel.tsxRepository: ArtisanPack-UI/visual-editor
Length of output: 3019
🏁 Script executed:
# Check if there are any __() calls elsewhere in blocks-panel.tsx
rg -n "__\(" resources/js/visual-editor/site-editor/styles/panels/blocks-panel.tsxRepository: ArtisanPack-UI/visual-editor
Length of output: 373
🏁 Script executed:
# Check the current state of styles-fields.tsx around the flagged lines
sed -n '150,170p' resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsx
sed -n '195,210p' resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsx
sed -n '255,270p' resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsx
sed -n '290,300p' resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsxRepository: ArtisanPack-UI/visual-editor
Length of output: 2877
Wrap descriptor labels with __() at declaration sites to enable translation extraction.
Using __(descriptor.label, TEXT_DOMAIN) with a variable at render time bypasses string extraction for translation catalogs. The labels in blocks-panel.tsx and elements-panel.tsx are hardcoded strings but are not wrapped with __() at declaration (unlike other UI strings in those files), preventing proper localization.
Update both:
- Descriptor
labelvalues inblocks-panel.tsxandelements-panel.tsxto wrap with__():label: __('Background color', TEXT_DOMAIN), - Remove
__()wrapping instyles-fields.tsx(lines 158, 202, 262, 296) to usedescriptor.labeldirectly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/visual-editor/site-editor/styles/panels/styles-fields.tsx` at
line 158, Update the label handling so translation extraction works: wrap the
hardcoded descriptor label values with __() where they are declared (update the
label fields in the descriptor objects inside blocks-panel.tsx and
elements-panel.tsx to use __('... ', TEXT_DOMAIN) for each label), and remove
the runtime wrapping in styles-fields.tsx so the render uses descriptor.label
directly (replace occurrences of __('descriptor.label', ...) /
__(descriptor.label, TEXT_DOMAIN) in styles-fields.tsx with just
descriptor.label). Target the descriptor objects named "label" in
blocks-panel.tsx and elements-panel.tsx and the render usage of descriptor.label
in styles-fields.tsx.
Previously the block / element detail panels declared descriptor labels
as bare string literals and wrapped them with __(descriptor.label,
TEXT_DOMAIN) at render time. The translator-extractor tooling only
scans for __('literal', ...) calls, so those labels never made it into
the catalog — the panel labels were stuck in English no matter which
locale the host app loaded.
Wrap each label at its declaration site instead, inside a getter
function (same pattern `getSiteEditorSections` uses) so the __() call
still runs post-bootI18n. styles-fields.tsx now consumes
`descriptor.label` directly since the string arriving is already
translated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
16 of 20 tasks
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.
Description
Builds the D3 Global Styles UI inside the D1 site-editor shell. Authors can now customize theme.json-shaped presets and styles — typography, colors, layout, per-block overrides, per-element overrides, and theme variations — and write them back through C3's
PUT /global-styles/{id}endpoint. The section mounts inside the shell's navigator / canvas / inspector slots using the same entity-state slot templates and template parts use, so the top-bar Save button ("Save global styles") wires up with no shell special-casing.Closes: #370
Type of Change
Related Issue
Issue: #370
Motivation and Context
Global styles is flagged in D0 as a known WP pain point (discoverability, unclear scope, buried Style Book). D3 implements D0's brief: Style Book is the default canvas, the navigator mirrors theme.json hierarchy, the inspector always shows scope breadcrumbs, and the variation picker lands in V1. Without D3, the Styles section shows only a placeholder; E1 (front-end CSS emission) is also blocked on this landing.
Changes Made
/lookup+/basedispatch into the core-data shim on mount (receiveCurrentGlobalStylesId,receiveGlobalStylesBase)/visual-editor/api/blocksso only registered block types appearStyles ▸ Blocks ▸ Button), dirty / saved / 422 chrome, and the six panels built entirely on@wordpress/componentsprimitives (PanelBody,PanelRow,SelectControl,TextControl,ColorPalette,ColorPicker+ColorIndicator+Dropdown,__experimentalUnitControl) so the Styles UI matches the block-inspector look & feelentityStateslot drives the top-bar Save button with label "Save global styles"; 422 responses surface field-level errors inlineHow Has This Been Tested?
Testing Environment:
Tests Performed:
ColorPickerpopover (not the native OS picker)Accessibility Tests Run
Details: navigator uses
role=\"tree\"with proper aria-expanded / aria-selected per node; inspector breadcrumb uses<nav>+aria-current=\"page\"; variation picker usesrole=\"radiogroup\"with roving tabindex so only the active (or first) radio is tab-reachable; every color / text control inherits Gutenberg's own focus + announcement behavior.Tests Added
Test details: 28 new tests across
styles/__tests__/styles-section.test.tsx+styles/__tests__/theme-json-paths.test.ts. Updatedsite-editor-app.test.tsxto stub the styles hook so shell tests don't reach the new REST endpoints. Full suite: 660 JS tests + 341 PHP tests passing.Documentation
Documentation details: Every module carries a header docblock explaining its role in the D1 shell + references to the design brief / plan sections it implements. Non-obvious choices (deterministic variation signature,
__experimentalUnitControlAPI risk, dual D2/D3 entity-state ownership) have inline comments.Screenshots
Added manually during manual-test review; palette editor now uses Gutenberg's
ColorPickerpopover; all rows inside a panel have visible separation via thePanelBodyrow dividers.Pre-Submission Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests