Skip to content

Fix editor validation errors on Services pill/tag badges (LS-4207) - #59

Closed
brandonmarshal wants to merge 4 commits into
developfrom
fix/services-badge-border-color-class
Closed

brandonmarshal wants to merge 4 commits into
developfrom
fix/services-badge-border-color-class

Conversation

@brandonmarshal

Copy link
Copy Markdown
Collaborator

Summary

The `Section - Services Linked Decisions` and `Section - Services Service Clusters` patterns on the Services page threw "Block contains unexpected or invalid content" with an "Attempt Recovery" prompt in the Site Editor, on the process-pill and cluster-tag badge blocks.

Both blocks set an inline border colour via a custom JSON style attribute (`style.border.color`), but WordPress's border block support always expects a matching `has-border-color` class on the block wrapper whenever a border colour is set. The hand-authored static HTML for these two patterns never included that class, so the editor's re-derived expected markup never matched the stored HTML, permanently flagging the blocks as invalid — even though they rendered correctly on the front end.

Confirmed via a working control case already in the codebase: `services-cta.php` sets a border colour the same way and correctly includes `has-border-color`, and shows no recovery error.

Fix

  • `patterns/sections/services-linked-decisions.php`: added `has-border-color` to the `ls-process-pill` wrapper class, matching the existing `has-background` convention
  • `patterns/sections/services-service-clusters.php`: added `has-border-color` to the `ls-cluster-tag` wrapper class, same convention

Not the cause, ruled out during investigation

An earlier, separate bug (color-mix() values silently dropped by the style engine) had caused similar recovery errors on the Hero pattern and was already fixed in a prior commit (`de2094e`). That fix does not apply here — the live page content for these two patterns has zero `color-mix()` occurrences, confirming this is a distinct, unrelated defect.

Test plan

  • `composer run lint:php` — both changed files pass with no syntax errors
  • Verified locally in the Site Editor (localhost) that the "Attempt Recovery" warning no longer appears on either pattern after the fix
  • Manual QA: re-verify front-end rendering is visually unchanged on staging/production

Closes LS-4207

Bug fix
- services-linked-decisions.php, services-service-clusters.php: add
  missing `has-border-color` class to the process-pill and
  cluster-tag wrapper groups
- Root cause: both blocks set an inline border color via a custom
  JSON style attribute, but WordPress's border block support always
  expects a `has-border-color` class on the wrapper whenever a
  border color is set — the hand-authored static HTML never
  included it, so the editor's re-derived expected markup never
  matched the stored HTML, permanently flagging these blocks as
  "unexpected or invalid content"
- Confirmed via a working control case in services-cta.php, which
  sets a border color the same way and already includes the class
  correctly
Docs
- CHANGELOG.md: add dated entry for the has-border-color fix on
  services-linked-decisions.php and services-service-clusters.php,
  matching the one-entry-per-PR convention already used for the
  other Services page and mobile-menu entries
@brandonmarshal brandonmarshal added area:theme Theme & styles (templates, template parts, FSE) comp:block-patterns Patterns library/registration lang:php PHP code priority:important High impact; schedule next. status:needs-review Awaiting code review labels Sep 17, 2026
@brandonmarshal brandonmarshal self-assigned this Sep 17, 2026
@linear-code

linear-code Bot commented Sep 17, 2026

Copy link
Copy Markdown

LS-4207

Docs
- CHANGELOG.md: add PR reference link now that the pull request
  exists, matching the existing per-PR link convention

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 38 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: cc80f8da-3860-4055-acfd-64fca56da51c

📥 Commits

Reviewing files that changed from the base of the PR and between 29f5095 and b5e7a15.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • patterns/sections/services-linked-decisions.php
  • patterns/sections/services-service-clusters.php

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@brandonmarshal

Copy link
Copy Markdown
Collaborator Author

This PR was accidentally closed by a branch-rename operation (fix/services-badge-border-color-classfix/services-badge-and-icon-attributes), which GitHub did not migrate automatically. All commits from this PR are preserved on the renamed branch. Superseded by #60, which carries this fix plus two additional small Icon-block fixes found in review.

brandonmarshal added a commit that referenced this pull request Sep 17, 2026
Docs
- CHANGELOG.md: expand the entry title/body to cover the Icon block
  width and slug fixes, and update the PR reference from #59 to #60
  after the accidental branch-rename close

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
brandonmarshal added a commit that referenced this pull request Sep 17, 2026
…tterns (LS-4207) (#60)

* Fix editor validation errors on Services pill/tag badges (LS-4207)

Bug fix
- services-linked-decisions.php, services-service-clusters.php: add
  missing `has-border-color` class to the process-pill and
  cluster-tag wrapper groups
- Root cause: both blocks set an inline border color via a custom
  JSON style attribute, but WordPress's border block support always
  expects a `has-border-color` class on the wrapper whenever a
  border color is set — the hand-authored static HTML never
  included it, so the editor's re-derived expected markup never
  matched the stored HTML, permanently flagging these blocks as
  "unexpected or invalid content"
- Confirmed via a working control case in services-cta.php, which
  sets a border color the same way and already includes the class
  correctly

* Add CHANGELOG entry for Services badge border-color fix (LS-4207)

Docs
- CHANGELOG.md: add dated entry for the has-border-color fix on
  services-linked-decisions.php and services-service-clusters.php,
  matching the one-entry-per-PR convention already used for the
  other Services page and mobile-menu entries

* Link PR #59 in CHANGELOG entry for LS-4207

Docs
- CHANGELOG.md: add PR reference link now that the pull request
  exists, matching the existing per-PR link convention

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix invalid Icon block width attribute and missing icon slug (LS-4207)

Bug fix
- section-card-services.php, services-service-tiles.php: move the
  Icon block's width from an unsupported top-level "width" JSON
  attribute to the correct "style":{"dimensions":{"width":...}}
  path, matching core/icon's actual schema — the old attribute was
  silently dropped, leaving the icon unsized
- section-card-services.php: fix icon slug "lightspeed/rocket-launch"
  to "lightspeed/rocket" — rocket-launch.svg does not exist in the
  ls-plugin icon library, so the icon failed to render entirely

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Update CHANGELOG entry for LS-4207 with icon fixes and PR #60 link

Docs
- CHANGELOG.md: expand the entry title/body to cover the Icon block
  width and slug fixes, and update the PR reference from #59 to #60
  after the accidental branch-rename close

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix false-positive console error on 404 template test (LS-2940)

Bug fix
- Chromium logs its own console.error for the main document's non-2xx
  status (e.g. loading the 404 template), independent of any theme JS
- browser-errors.ts had no exemption for this, so the standing suite's
  404-page test always failed even though the page behaves correctly
- network-errors.ts already exempts the equivalent main-frame response
  for the same reason; mirrored that exemption on the console side

Context
- Confirmed via retest that this was the only remaining console error
  under LS-2940 — the other 4 flagged pages (#244, #246, #247, #248)
  were already resolved by the earlier ls-plugin CSS fix (LS-2935)

* Fix false-positive suppression risk in browser-errors status check (LS-2940)

Bug fix
- browser-errors.ts: replace the URL-comparison exemption with a
  main-frame navigation response check
- Root cause: comparing the failed resource's URL to the page's own
  URL could wrongly suppress a real subresource error if that
  subresource happened to request the exact same URL as the page
- Now tracks the actual main-frame navigation response's status via
  isNavigationRequest() + frame() === page.mainFrame(), reset on
  every navigation, so only that navigation's real status is exempt

Context
- Flagged by CodeRabbit on PR #60 as a minor hardening suggestion,
  unrelated to that PR's LS-4207 scope but fixed here since it was
  already part of the branch
- Verified locally: special-routes.spec.ts passes on Chromium
  (including the 404-template test this logic targets), no
  regressions in page-structure.spec.ts (its one failure is
  pre-existing and unrelated — missing <main> landmark/heading
  hierarchy, not console-error handling)

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:theme Theme & styles (templates, template parts, FSE) comp:block-patterns Patterns library/registration lang:php PHP code priority:important High impact; schedule next. status:needs-review Awaiting code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant