Work Archive Template: Audit & Fix (LS-2244) - #19
Open
brandonmarshal wants to merge 7 commits into
Open
Conversation
…cross work-related components
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ilter spacing Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rk-archive-template-audit-fix
…grid; update text for clarity and add Playwright test helpers for improved testing coverage.
- Add hover/focus-within lift, border, and shadow to is-style-card-case-study (Selected Projects cards), matching the existing card-motion treatment used elsewhere on the page - Add surface.canvas-alt token (theme.json + styles/dark.json) with new neutral-150/neutral-850 palette swatches, verified against WCAG AA 2.2 for text tokens rendered directly on section bands - Fix text.brand contrast: swap brand-500 for brand-600 so it clears 4.5:1 AA on canvas, card, and canvas-alt backgrounds - Add content-band-alt style variation (canvas-alt background, 16px radius) and apply it to the Selected Projects and Discuss Project sections for an alternating section rhythm down the page
brandonmarshal
added
area:design-system
|
Warning Review limit reached
Next review available in: 37 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
Note
|
| Layer / File(s) | Summary |
|---|---|
Theme and style foundations styles/blocks/..., styles/dark.json, styles/sections/..., theme.json |
Adds alternate content-band, divider, and surface styles. Adds neutral palette tokens and updates surface and brand color mappings. |
Statistics layout patterns/section-stats-grid.php, patterns/cards/work-engagement-stat.php |
Reorganizes statistics into nested column groups with borders, wrapping labels, updated alignment, and revised supporting copy. |
Work pattern updates patterns/hero/..., patterns/sections/..., patterns/cards/work-next-steps-card.php |
Updates Work section styles, eyebrow icons, selected-project filtering, card alignment, and column sizing. |
Motion and responsive behavior src/scss/animations/..., src/scss/sections/..., assets/css/animations.css |
Adds case-study card hover and focus motion with reduced-motion overrides. Adds responsive rules for statistics and selected-project grids. |
Work page validation tests/helpers/assertions.ts, tests/specs/work-single.spec.ts |
Adds reusable Playwright assertions and Work Single coverage for structure, card content, navigation, responsive grids, and computed styles. |
Estimated code review effort: 4 (Complex) | ~45 minutes
Possibly related PRs
- lightspeedwp/ls-theme#15: Introduces the Work Archive patterns and components refined by this PR.
- lightspeedwp/ls-theme#3: Shares theme color tokens and animation styling changes.
- lightspeedwp/ls-theme#4: Covers modular theme configuration related to the updated preset values.
Suggested labels: lang:js, release:patch
Suggested reviewers: zaredrogers
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly identifies the Work Archive Template audit and fixes, which match the primary objectives of the pull request. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/ls-2244-work-archive-template-audit-fix
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 help to get the list of available commands.
|
Tick the box to add this pull request to the merge queue (same as
|
There was a problem hiding this comment.
Pull request overview
This PR updates the LightSpeed Theme’s Work archive section patterns and supporting styles/tokens to resolve recurring block markup issues, improve responsive layout/styling (Selected Projects + Stats Grid), and introduce a new alternate “content band” surface treatment. It also adds generic Playwright assertion helpers plus an example spec intended to demonstrate how those helpers would be used for a future Work Single template.
Changes:
- Extended
theme.jsoncolour palette and semantic colour mappings (incl. newsurface.canvas-altand improvedtext.brandcontrast), with a matching dark-mode semantic mapping. - Added/relocated block/style-variation JSON and Sass overrides to fix Stats Grid borders/heights and introduce a custom 3→2→1 and 4→2 responsive behaviour where core attributes are insufficient.
- Added Playwright assertion helpers and an example Work Single spec.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
theme.json |
Adds new palette steps and updates semantic colour mappings (brand + canvas-alt). |
styles/dark.json |
Adds dark-mode mapping for the new surface.canvas-alt semantic token. |
styles/sections/content/content-band-alt.json |
Introduces a section style variation using the new canvas-alt surface. |
styles/blocks/groups/stat-segment.json |
Adjusts the Stat Segment style variation (removes inline css). |
styles/blocks/columns/card-divider-both.json |
Adds a Columns style variation to standardise top/bottom dividers and equal-height behaviour. |
src/scss/sections/_stats-grid.scss |
Adds a custom breakpoint override for the Stats Grid layout and divider behaviour. |
src/scss/sections/_selected-projects.scss |
Adds an intermediate 2-column breakpoint for the Selected Projects query grid. |
src/scss/animations/_card-motion.scss |
Adds hover/focus-within motion treatment for case-study cards + reduced-motion handling. |
src/scss/animations.scss |
Wires the new section Sass partials into the compiled CSS output. |
assets/css/animations.css |
Compiled CSS output for the new motion + section breakpoint rules. |
patterns/sections/work-selected-projects.php |
Applies the new content band alt section style and updates filter/query markup. |
patterns/sections/work-related-routes.php |
Adjusts layout alignment and updates eyebrow/icon markup. |
patterns/sections/work-discuss-project.php |
Applies the new content band alt section style and adjusts columns sizing/markup. |
patterns/sections/work-categories.php |
Replaces the eyebrow badge pattern with inline eyebrow/icon markup. |
patterns/section-stats-grid.php |
Rebuilds the Stats Grid structure to fix borders/heights and breakpoint behaviour. |
patterns/hero/work-hero.php |
Updates eyebrow/icon markup in the Work archive hero. |
patterns/cards/work-next-steps-card.php |
Updates card layout alignment to vertically centre icon/content. |
patterns/cards/work-engagement-stat.php |
Updates segment markup (border + spacing + copy). |
tests/helpers/assertions.ts |
Adds reusable Playwright assertion helpers for template/pattern QA. |
tests/specs/work-single.spec.ts |
Adds an example Playwright spec for a future Work Single template. |
Suppressed comments (1)
tests/helpers/assertions.ts:118
getPropertyValue()often returns values with leading/trailing whitespace; comparing untrimmed strings makes this assertion brittle across browsers/properties.
const value = await page
.locator(selector)
.first()
.evaluate((el, prop) => getComputedStyle(el).getPropertyValue(prop), property);
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/scss/sections/_stats-grid.scss (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared breakpoint mixin in both section partials.
Both new partials add raw media queries instead of using the repository breakpoint abstraction.
- src/scss/sections/_stats-grid.scss#L15-L15: express the 833.98px threshold through the shared
mqmixin.- src/scss/sections/_selected-projects.scss#L12-L12: express the 601px-to-800px interval through the shared
mqmixin.As per path instructions,
src/scss/**/*.scssmust reuse the shared breakpoint mixin.🤖 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 `@src/scss/sections/_stats-grid.scss` at line 15, Replace the raw media query in src/scss/sections/_stats-grid.scss:15 with the shared mq mixin for the 833.98px threshold. Also update src/scss/sections/_selected-projects.scss:12 to express the 601px-to-800px interval through mq, preserving each partial’s existing responsive behavior.Source: Path instructions
🤖 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 `@patterns/sections/work-discuss-project.php`:
- Around line 22-23: Update the wp:columns block markup to keep its serialized
style.spacing.blockGap and rendered gap styling consistent: either restore the
corresponding rendered gap style or remove the stale style object from the block
comment, while preserving the intended layout.
In `@src/scss/animations/_card-motion.scss`:
- Around line 249-255: Add an empty line between the custom properties and the
transition declaration in the case-study rule in
src/scss/animations/_card-motion.scss (lines 249-255), then regenerate the
corresponding compiled rule in assets/css/animations.css (lines 409-414) so both
sources satisfy Stylelint and remain synchronized.
In `@src/scss/sections/_selected-projects.scss`:
- Around line 12-14: The responsive grid breakpoint is inconsistent with the
intended 600.01px lower bound. Update the media query surrounding
.work-selected-projects-grid in src/scss/sections/_selected-projects.scss:12-14
to use 600.01px or the shared equivalent token, then regenerate
assets/css/animations.css:2032-2034 from the corrected source so both files
match.
In `@src/scss/sections/_stats-grid.scss`:
- Line 7: Remove the standalone empty SCSS comment from the stylesheet, leaving
the surrounding styles unchanged.
- Around line 24-25: Update the responsive override in
src/scss/sections/_stats-grid.scss at lines 24-25 to use border-inline-end: none
!important instead of border-right, reducing the selector depth where possible
while preserving its target behavior. Regenerate assets/css/animations.css at
lines 2028-2029 so the compiled override uses the same logical property.
In `@tests/helpers/assertions.ts`:
- Around line 46-49: Update expectCardParts() to verify that each required part
is present without requiring exactly one match. Replace the toHaveCount(1)
assertion on card.locator(part) with the appropriate presence assertion while
preserving the existing timeout and diagnostic message.
In `@tests/specs/work-single.spec.ts`:
- Around line 11-17: Replace the placeholder content in the work-single spec
with the finalized Work Single template URL, selectors, and expected UI
assertions. Update the six helper usages to target real elements and remove the
illustrative comments and guessed constants, ensuring the test provides actual
regression coverage.
---
Nitpick comments:
In `@src/scss/sections/_stats-grid.scss`:
- Line 15: Replace the raw media query in src/scss/sections/_stats-grid.scss:15
with the shared mq mixin for the 833.98px threshold. Also update
src/scss/sections/_selected-projects.scss:12 to express the 601px-to-800px
interval through mq, preserving each partial’s existing responsive behavior.
🪄 Autofix
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.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: da1b6337-2eb9-4d3a-848e-7f30c579b483
📒 Files selected for processing (20)
assets/css/animations.csspatterns/cards/work-engagement-stat.phppatterns/cards/work-next-steps-card.phppatterns/hero/work-hero.phppatterns/section-stats-grid.phppatterns/sections/work-categories.phppatterns/sections/work-discuss-project.phppatterns/sections/work-related-routes.phppatterns/sections/work-selected-projects.phpsrc/scss/animations.scsssrc/scss/animations/_card-motion.scsssrc/scss/sections/_selected-projects.scsssrc/scss/sections/_stats-grid.scssstyles/blocks/columns/card-divider-both.jsonstyles/blocks/groups/stat-segment.jsonstyles/dark.jsonstyles/sections/content/content-band-alt.jsontests/helpers/assertions.tstests/specs/work-single.spec.tstheme.json
- Remove 5 hardcoded iconColor/iconColorValue attributes; they already rendered via the icon.background custom token (light + dark values) - Restore missing block-gap inline style on work-discuss-project's wp:columns wrapper to match its serialized attribute - Fix expectCardParts() to check presence instead of an exact count of 1 - Remove unused Locator import and trim getPropertyValue() output in tests/helpers/assertions.ts - Skip the illustrative work-single.spec.ts suite until the template exists - Fix two Stylelint violations and switch to border-inline-end for RTL safety in the SCSS section partials - Rebuild assets/css/animations.css from the corrected SCSS sources Not applied (3 recommendations): - CodeRabbit suggested reverting the Selected Projects breakpoint from 601px back to 600.01px. Declined: 600.01px was already tried and caused a devicePixelRatio:2 rounding bug where it and WordPress's native ≤600px query matched simultaneously at reported width 600, breaking the native 1-column mobile layout. 601px was the verified fix. - CodeRabbit suggested routing the 833.98px and 601–800px breakpoints through the shared mq() Sass mixin. Declined: mq() only supports named breakpoints from the theme's $ls-breakpoints map, and neither of these custom pixel values exists in that map — using raw @media queries was the deliberate workaround for that exact limitation, not an oversight. - Copilot suggested the grid-template-columns column count in expectGridColumnsAtViewport() is unreliable because minmax() isn't split correctly. Declined: getComputedStyle().gridTemplateColumns is a CSSOM special case that returns resolved pixel tracks, not the minmax()/repeat() source syntax, so splitting on spaces is correct — confirmed by this session's own live browser testing.
ZaredRogers
reviewed
Aug 6, 2026
| --ls-card-case-study-border-active: var(--wp--custom--color--link--accent); | ||
| --ls-card-case-study-shadow-active: var(--wp--custom--shadow--card--hover); | ||
| --ls-card-case-study-lift: -4px; | ||
|
|
Contributor
There was a problem hiding this comment.
@brandonmarshal, these classes the agent keeps adding just to reference a variable and call once, is there a reason why it's doing this? Or is this just a wrong path it's gone down. I recommend asking it.
If it is wrong, I imagine, unfortunately, that it may be quite prevalent, I've seen it twice in this PR already.
I would also recommend defining some default "lift" values as custom variables in theme.json that it can use in animations, like the animation duration variables
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.
Summary
archive-worktemplate and its section patterns for broken/recurring paragraph block errors (LS-2244) and rebuilt the affected blocks to match approved DEV markuptests/helpers/assertions.ts) extracted from manually QA-testing this template, plus an example spec for the upcoming work-single templatesurface.canvas-altdesign token (light + dark values, WCAG AA 2.2 verified) and acontent-band-altstyle variation, applied to the Selected Projects and Discuss Project sections for an alternating section background rhythmtext.brandcontrast gap by swappingbrand-500forbrand-600so it clears 4.5:1 AA against canvas, card, and canvas-alt backgroundsCloses LS-2244
Test plan
surface.canvas-altandtext.brandcontrast ratios against WCAG AA 2.2 (4.5:1) for all text tokens used directly on section bandsnpm run build:csscompiles cleanlySummary by CodeRabbit
New Features
Bug Fixes
Tests