SEO: migrate Settings styles to CSS Modules - #50619
Merged
Merged
Conversation
gmjuhasz
added
[Status] Needs Review
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Package] Seo
[Status] In Progress
[Tests] Includes Tests
labels
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
gmjuhasz
force-pushed
the
agent/migrate-seo-settings-styles
branch
from
July 21, 2026 09:24
9320709 to
aaf4aa6
Compare
gmjuhasz
force-pushed
the
agent/migrate-seo-settings-styles
branch
from
July 21, 2026 10:27
aaf4aa6 to
ec4bcf8
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the SEO package’s migration away from global Settings SCSS by moving the Settings screen to scoped CSS Modules and reusing WPUI layout/typography components, while aiming to preserve existing behavior and accessibility.
Changes:
- Replaced global Settings SCSS usage with per-screen/per-component CSS Modules.
- Refactored Settings screen markup to use WPUI
Stack,Text, andLinkfor layout/typography. - Updated Schema Settings tests to work with CSS Modules class names (but dropped some style-contract assertions).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/packages/seo/changelog/migrate-seo-styles-to-css-modules | Adds changelog metadata entry for the Settings CSS Modules migration. |
| projects/packages/seo/_inc/screens/settings/index.tsx | Migrates Settings screen wrapper/layout to Stack and CSS module classes. |
| projects/packages/seo/_inc/screens/settings/style.scss | Removes legacy global Settings SCSS. |
| projects/packages/seo/_inc/screens/settings/style.module.scss | Adds scoped module styles for Settings root/section + sitemap hint/link. |
| projects/packages/seo/_inc/screens/settings/verification-card.tsx | Drops global SCSS import and replaces grid wrapper with Stack. |
| projects/packages/seo/_inc/screens/settings/google-verification-field.tsx | Reworks layout/typography with WPUI components and removes unused wrapper class. |
| projects/packages/seo/_inc/screens/settings/title-structure-field.tsx | Migrates title-structure rows to CSS Modules and uses Text for muted/preview text. |
| projects/packages/seo/_inc/screens/settings/title-structure-field.module.scss | Adds module styles for title-structure row separators, preview flexing, and Save alignment. |
| projects/packages/seo/_inc/screens/settings/social-previews-card.tsx | Switches preview/icon styling to CSS Modules. |
| projects/packages/seo/_inc/screens/settings/social-previews-card.module.scss | Adds scoped styles for preview cards/icons, including nested selectors. |
| projects/packages/seo/_inc/screens/settings/schema-card.tsx | Removes legacy global SCSS import (Schema styles now live in schema-settings module). |
| projects/packages/seo/_inc/screens/settings/author-profile-card.tsx | Removes legacy global SCSS import. |
| projects/packages/seo/_inc/screens/settings/schema-settings/style.module.scss | Introduces CSS Modules for Schema settings shared layout/error styles. |
| projects/packages/seo/_inc/screens/settings/schema-settings/profile-url-list.tsx | Migrates labels/help and error styling to WPUI + CSS Modules. |
| projects/packages/seo/_inc/screens/settings/schema-settings/organization-section.tsx | Migrates muted text and Save-row alignment to WPUI + CSS Modules. |
| projects/packages/seo/_inc/screens/settings/schema-settings/local-business-section.tsx | Replaces legacy Save wrapper class with Stack alignment. |
| projects/packages/seo/_inc/screens/settings/schema-settings/local-business-fields.tsx | Migrates paired-field grids, errors, and opening-hours layout to CSS Modules + WPUI Text/Stack. |
| projects/packages/seo/_inc/screens/settings/schema-settings/author-profile-section.tsx | Migrates muted text, avatar row, and external link to WPUI Text/Stack/Link + CSS Modules. |
| projects/packages/seo/_inc/screens/settings/test/schema-card.test.tsx | Updates some CSS-selector assertions for modules; removes pairError assertions that should be retained. |
gmjuhasz
marked this pull request as ready for review
July 22, 2026 08:08
…ttings-styles-v1 # Conflicts: # projects/packages/seo/_inc/screens/settings/title-structure-field.tsx
github-actions
Bot
removed
[Status] In Progress
[Status] Needs Review
angelablake
pushed a commit
that referenced
this pull request
Jul 30, 2026
Findings from /jetpack-review-pr plus a five-lens adversarial battery (core, simplifier, unit-test audit, a11y/RTL, docs/copy). Every test claim below was verified by mutation — breaking the production code and confirming a test fails. Correctness: - An empty profile row no longer counts toward completion. "Add profile" seeds `sameAs` with `''`, which made the header read "Complete" while nothing was saved and Save stayed disabled, because `cleanOrganization` strips the row so the form never becomes dirty. Now only non-blank entries count. - The completion threshold is derived from the field list instead of a literal `3`, so adding a field can't silently stop the module ever reading Complete. Accessibility: - The Organization details and their local-business refinement are wrapped in a real `Fieldset.Root` with a visually-hidden legend. Merging three cards into one had left them as flat siblings whose grouping existed only in a visual rule; a `Stack` is a plain div and carries no semantics. - That fieldset also draws the hairline, replacing the `<hr>`. An `<hr>` maps to an unnamed `separator` node, so it announced a purely visual cue. A border on the container is the pattern `title-structure-field` already uses. - The Save button says why it's disabled and points at the message with `aria-describedby`. One Save covers both sections now, so an error in either blocks it — and the button stays focusable while disabled, because `@wordpress/ui` sets `aria-disabled` rather than the native attribute. - The Organization "Name" field gets an accessible name of "Organization name": the Author profile card on the same tab also has a "Name" field, a collision `settings/index.tsx` already documents avoiding. Simplification: - `commitBreadcrumbList` and `persist` were the same function — guard, notice, POST, re-seed, error notice, clear the flag. Breadcrumbs now goes through `persist` with a notice string and an optional rollback callback, which also gives the explicit Save a specific "Saving schema settings…" instead of the bare "Saving…" a reviewer had already pushed back on in #50541. - Comments that restated the code beside them are gone; one that still described the deleted per-section Save is corrected. Test coverage — all of these were mutation-verified as absent: - The `isSaving` arm of the Save's disabled expression, and the Breadcrumbs toggle's own `disabled`. Both could be deleted with all tests still green. - A positive path through both validators. Neither had one, so a validator that rejected everything — leaving Save permanently stuck — would have shipped green. Covers valid international details, the 99-char price-range boundary, and overnight hours. - Duplicate profile-URL detection, which had no coverage anywhere in the suite. - The `localBusiness.enabled &&` short-circuit, the explicit-value half of the status rule, the Breadcrumbs off state, and email's exclusion from the status. - Restored two assertions the rewrite dropped: the long-s country code (the reason `uppercaseAscii` exists instead of `.toUpperCase()`) and the opening-hours `aria-describedby` pair contract asked for in #50619. - The SCSS mock now lists every class the components read; a missing key resolves to `undefined` and silently passes `toHaveClass`. Docs and copy: - "map results" overclaimed — LocalBusiness markup drives rich results, while Maps placement comes from a Google Business Profile. - Bare "AI" is now "AI assistants", matching the rest of the package. - README described the Author profile card as per-author; it edits the signed-in user only. Same correction in the card's docblock. - Changelog drops the design-system jargon, and the Breadcrumbs rollback gets its own entry as a user-visible fix. 235 JS tests pass, up from 225. Typecheck, ESLint and stylelint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
angelablake
pushed a commit
that referenced
this pull request
Jul 30, 2026
Findings from /jetpack-review-pr plus a five-lens adversarial battery (core, simplifier, unit-test audit, a11y/RTL, docs/copy). Every test claim below was verified by mutation — breaking the production code and confirming a test fails. Correctness: - An empty profile row no longer counts toward completion. "Add profile" seeds `sameAs` with `''`, which made the header read "Complete" while nothing was saved and Save stayed disabled, because `cleanOrganization` strips the row so the form never becomes dirty. Now only non-blank entries count. - The completion threshold is derived from the field list instead of a literal `3`, so adding a field can't silently stop the module ever reading Complete. Accessibility: - The Organization details and their local-business refinement are wrapped in a real `Fieldset.Root` with a visually-hidden legend. Merging three cards into one had left them as flat siblings whose grouping existed only in a visual rule; a `Stack` is a plain div and carries no semantics. - That fieldset also draws the hairline, replacing the `<hr>`. An `<hr>` maps to an unnamed `separator` node, so it announced a purely visual cue. A border on the container is the pattern `title-structure-field` already uses. - The Save button says why it's disabled and points at the message with `aria-describedby`. One Save covers both sections now, so an error in either blocks it — and the button stays focusable while disabled, because `@wordpress/ui` sets `aria-disabled` rather than the native attribute. - The Organization "Name" field gets an accessible name of "Organization name": the Author profile card on the same tab also has a "Name" field, a collision `settings/index.tsx` already documents avoiding. Simplification: - `commitBreadcrumbList` and `persist` were the same function — guard, notice, POST, re-seed, error notice, clear the flag. Breadcrumbs now goes through `persist` with a notice string and an optional rollback callback, which also gives the explicit Save a specific "Saving schema settings…" instead of the bare "Saving…" a reviewer had already pushed back on in #50541. - Comments that restated the code beside them are gone; one that still described the deleted per-section Save is corrected. Test coverage — all of these were mutation-verified as absent: - The `isSaving` arm of the Save's disabled expression, and the Breadcrumbs toggle's own `disabled`. Both could be deleted with all tests still green. - A positive path through both validators. Neither had one, so a validator that rejected everything — leaving Save permanently stuck — would have shipped green. Covers valid international details, the 99-char price-range boundary, and overnight hours. - Duplicate profile-URL detection, which had no coverage anywhere in the suite. - The `localBusiness.enabled &&` short-circuit, the explicit-value half of the status rule, the Breadcrumbs off state, and email's exclusion from the status. - Restored two assertions the rewrite dropped: the long-s country code (the reason `uppercaseAscii` exists instead of `.toUpperCase()`) and the opening-hours `aria-describedby` pair contract asked for in #50619. - The SCSS mock now lists every class the components read; a missing key resolves to `undefined` and silently passes `toHaveClass`. Docs and copy: - "map results" overclaimed — LocalBusiness markup drives rich results, while Maps placement comes from a Google Business Profile. - Bare "AI" is now "AI assistants", matching the rest of the package. - README described the Author profile card as per-author; it edits the signed-in user only. Same correction in the card's docblock. - Changelog drops the design-system jargon, and the Breadcrumbs rollback gets its own entry as a user-visible fix. 235 JS tests pass, up from 225. Typecheck, ESLint and stylelint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
angelablake
added a commit
that referenced
this pull request
Jul 30, 2026
…ETPACK-2079] (#50918) * SEO: combine the site-level schema controls into one Schema module [JETPACK-2079] The Settings tab showed Breadcrumbs, Organization, and Local business as three sibling cards, each with its own Save. LocalBusiness isn't a peer of Organization though — schema.org models it as a kind of Organization — so the three cards presented a hierarchy that doesn't exist and made an admin click Save twice to describe one entity. Collapse them into a single "Schema" module: - One CollapsibleCard with the shared `code` icon chip and the 3-state StatusIndicator that #50883 rolled out to the other Settings modules. Status follows the agreed rule — a field counts when it has a value OR its smart default — so a site with a Site Title and Tagline reads "In progress" and adding social/profile links completes it. - Breadcrumbs stays an auto-saving toggle at the top, separated from the entity details by a hairline rule. - Local business becomes a sub-toggle under the Organization fields. - One Save at the bottom persists the Organization fields and the LocalBusiness refinement together, via a new `saveOrganizationEntity()`. The two per-section save paths collapse into a shared `persist()` helper. - The Breadcrumbs auto-save now rolls the toggle back when the request fails, and ignores a toggle while another save is in flight, so the UI can't assert a state the server never accepted. Copy pass on the module and Organization fields, and the doubled intro paragraph is trimmed to the one non-obvious fact (the logo comes from the Site Logo / Site Icon). No PHP changes: the REST route already merges partial section payloads, and its sanitizers normalize rather than reject, so the combined POST can't be turned away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * SEO: address the pre-review battery on the Schema module Findings from /jetpack-review-pr plus a five-lens adversarial battery (core, simplifier, unit-test audit, a11y/RTL, docs/copy). Every test claim below was verified by mutation — breaking the production code and confirming a test fails. Correctness: - An empty profile row no longer counts toward completion. "Add profile" seeds `sameAs` with `''`, which made the header read "Complete" while nothing was saved and Save stayed disabled, because `cleanOrganization` strips the row so the form never becomes dirty. Now only non-blank entries count. - The completion threshold is derived from the field list instead of a literal `3`, so adding a field can't silently stop the module ever reading Complete. Accessibility: - The Organization details and their local-business refinement are wrapped in a real `Fieldset.Root` with a visually-hidden legend. Merging three cards into one had left them as flat siblings whose grouping existed only in a visual rule; a `Stack` is a plain div and carries no semantics. - That fieldset also draws the hairline, replacing the `<hr>`. An `<hr>` maps to an unnamed `separator` node, so it announced a purely visual cue. A border on the container is the pattern `title-structure-field` already uses. - The Save button says why it's disabled and points at the message with `aria-describedby`. One Save covers both sections now, so an error in either blocks it — and the button stays focusable while disabled, because `@wordpress/ui` sets `aria-disabled` rather than the native attribute. - The Organization "Name" field gets an accessible name of "Organization name": the Author profile card on the same tab also has a "Name" field, a collision `settings/index.tsx` already documents avoiding. Simplification: - `commitBreadcrumbList` and `persist` were the same function — guard, notice, POST, re-seed, error notice, clear the flag. Breadcrumbs now goes through `persist` with a notice string and an optional rollback callback, which also gives the explicit Save a specific "Saving schema settings…" instead of the bare "Saving…" a reviewer had already pushed back on in #50541. - Comments that restated the code beside them are gone; one that still described the deleted per-section Save is corrected. Test coverage — all of these were mutation-verified as absent: - The `isSaving` arm of the Save's disabled expression, and the Breadcrumbs toggle's own `disabled`. Both could be deleted with all tests still green. - A positive path through both validators. Neither had one, so a validator that rejected everything — leaving Save permanently stuck — would have shipped green. Covers valid international details, the 99-char price-range boundary, and overnight hours. - Duplicate profile-URL detection, which had no coverage anywhere in the suite. - The `localBusiness.enabled &&` short-circuit, the explicit-value half of the status rule, the Breadcrumbs off state, and email's exclusion from the status. - Restored two assertions the rewrite dropped: the long-s country code (the reason `uppercaseAscii` exists instead of `.toUpperCase()`) and the opening-hours `aria-describedby` pair contract asked for in #50619. - The SCSS mock now lists every class the components read; a missing key resolves to `undefined` and silently passes `toHaveClass`. Docs and copy: - "map results" overclaimed — LocalBusiness markup drives rich results, while Maps placement comes from a Google Business Profile. - Bare "AI" is now "AI assistants", matching the rest of the package. - README described the Author profile card as per-author; it edits the signed-in user only. Same correction in the card's docblock. - Changelog drops the design-system jargon, and the Breadcrumbs rollback gets its own entry as a user-visible fix. 235 JS tests pass, up from 225. Typecheck, ESLint and stylelint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * SEO: measure Schema completion against what would actually be stored Copilot review on #50918. Extends the empty-profile-row fix: the status was still counting values that server sanitization is about to discard. - Profile links now run through the same `cleanProfileUrls` the save path uses, which drops blank, malformed and duplicate rows. A single invalid URL used to read as a configured link, so the header could say "Complete" while Save sat disabled on that very validation error. - Name and description are trimmed before counting, so a whitespace-only override no longer reads as configured. Both are the same defect as the empty row: the header asserting a state the admin cannot actually save. Two tests, covering an invalid link, a duplicate pair collapsing to one counted link, and whitespace-only text. 237 JS tests pass, up from 235. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * SEO: send only the schema sections that changed [JETPACK-2079] `saveOrganizationEntity` posted both `organization` and `localBusiness` on every click, whichever one you had edited. That widened the lost-update window from one section to two: with the Schema settings open in two tabs, saving Organization in one would overwrite a local-business edit saved from the other with this tab's stale copy. Before the single Save, an Organization save sent `organization` alone and couldn't touch it. The request now carries only the dirty sections. The route already merges partial payloads — the property that made the combined Save safe in the first place — so an untouched section keeps exactly what is stored. The success handler still re-seeds both sections from the response. Neither is dirty by then, so there are no pending edits to lose, and a section changed elsewhere is picked up rather than silently ignored — which is a small improvement on the previous behaviour too. Found by re-deriving the risk rather than by the review battery or Copilot, neither of which flagged it. Tests: 240. The existing payload-shape assertion caught the change immediately, and a new parameterised case pins all three combinations (Organization alone, local business alone, both). Reverting to sending both unconditionally fails three tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Angela Blake <angela.blake@a8c.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Proposed changes
Stack,Text, andLinkfor applicable layout and typography while preserving semantic elements, IDs, ARIA relationships, behavior, accessibility, settings data flow, and public APIs.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
Automated checks:
git diff --check.pnpm exec eslint projects/packages/seo.pnpm exec stylelint 'projects/packages/seo/_inc/**/*.{css,scss,sass}'.pnpm --filter @automattic/jetpack-seo run typecheck.pnpm --filter @automattic/jetpack-seo run test --runInBand.jp build packages/seo --deps.Manual checks in Jetpack → SEO → Settings: