refact: continue fixes 4 #256
Conversation
- Removed reference from layouts/page/single.html - Deleted themes/beaver/assets/css/fl-component-layout-alt.css - Phase 1 FL CSS removal: 21KB saved - Tests: 39 runs, 57 assertions, 0 failures
The removal of .fl-node-2il86phfbmex CSS in commit 8397a2b broke service page layouts. This fix replaces the FL-Builder node reference with inline styles maintaining the same visual behavior (width: 100%). - Updated line 37 in themes/beaver/layouts/services/single.html - Replaced fl-node-2il86phfbmex with inline CSS equivalent - Maintains backward compatibility with FL-Builder structure - Fixes visual regression tests for service pages Tests now pass: 23 runs, 36 assertions, 0 failures 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
WalkthroughAdds BEM-based component CSS (c-hero, c-card, c-content), introduces critical and foundation FL CSS, removes legacy fl-component-layout-alt.css and its layout reference, tweaks a services layout column, and adds two planning docs detailing migration and CSS optimization strategies. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
themes/beaver/layouts/services/single.html (1)
37-75: Reintroduce the node-specific class for this columnLine 37 drops the
fl-node-2il86phfbmexidentifier. Those FL node IDs are still referenced by the dynamic CSS bundle we assemble via$servicesResources(notably the template-driven asset at Line 3), so removing the class severs the responsive spacing/width rules scoped to that column. The inline style you added covers only the base desktop flow; tablet/mobile overrides and any node-specific tweaks are now lost. Please keep the node class (and add the inline style alongside it if needed).- <div class="fl-col" style="width: 100%; float: left; min-height: 1px;"> + <div class="fl-col fl-node-2il86phfbmex" style="width: 100%; float: left; min-height: 1px;">
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/component-migration-plan.md(1 hunks)docs/css-optimization-findings.md(1 hunks)themes/beaver/assets/css/components/blocks/c-card.css(1 hunks)themes/beaver/assets/css/components/blocks/c-content.css(1 hunks)themes/beaver/assets/css/components/blocks/c-hero.css(1 hunks)themes/beaver/assets/css/critical.css(1 hunks)themes/beaver/assets/css/fl-component-layout-alt.css(0 hunks)themes/beaver/assets/css/fl-foundation.css(1 hunks)themes/beaver/layouts/page/single.html(0 hunks)themes/beaver/layouts/services/single.html(1 hunks)
💤 Files with no reviewable changes (2)
- themes/beaver/assets/css/fl-component-layout-alt.css
- themes/beaver/layouts/page/single.html
🧰 Additional context used
📓 Path-based instructions (2)
themes/**/assets/**/*.{css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
CSS/SCSS should follow project performance and responsiveness practices; mobile @media queries must avoid problematic CSS variable rendering (use direct hex if needed)
Files:
themes/beaver/assets/css/components/blocks/c-card.cssthemes/beaver/assets/css/components/blocks/c-content.cssthemes/beaver/assets/css/fl-foundation.cssthemes/beaver/assets/css/critical.cssthemes/beaver/assets/css/components/blocks/c-hero.css
{layouts,themes/**/layouts}/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
Hugo templates must use Go template syntax and follow Hugo layout patterns (no foreign templating or React/JSX)
Files:
themes/beaver/layouts/services/single.html
🧬 Code graph analysis (1)
themes/beaver/assets/css/critical.css (1)
test/unit/hugo_partials_test.rb (1)
test_critical_css_partial(168-185)
🪛 markdownlint-cli2 (0.18.1)
docs/component-migration-plan.md
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_deploy / build
| ``` | ||
| themes/beaver/assets/css/components/ | ||
| ├── base/ | ||
| │ ├── c-grid.css | ||
| │ └── c-section.css | ||
| ├── layout/ | ||
| │ ├── c-nav.css | ||
| │ └── c-footer.css | ||
| └── blocks/ | ||
| ├── c-hero.css | ||
| ├── c-card.css | ||
| ├── c-content.css | ||
| ├── c-testimonial.css | ||
| ├── c-cta.css | ||
| └── c-form.css (existing) | ||
| ``` |
There was a problem hiding this comment.
Add a language to this fenced code block
Markdownlint (MD040) flags the bare triple backtick fence around the directory tree. Add a language identifier (e.g., text) so the docs pipeline passes.
-```
+```text
themes/beaver/assets/css/components/
├── base/
│ ├── c-grid.css🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In docs/component-migration-plan.md around lines 138 to 153, the fenced code
block showing the directory tree is missing a language identifier which triggers
markdownlint MD040; update the opening triple-backtick fence to include a
language (for example "text") so it becomes ```text and leave the block contents
unchanged, then save the file so the docs pipeline no longer flags the block.
| .c-card--clickable { | ||
| cursor: pointer; | ||
| text-decoration: none; | ||
| display: block; | ||
| } |
There was a problem hiding this comment.
Restore flex layout for clickable cards
.c-card--clickable overrides the base .c-card display:flex with display:block, so the layout loses flex behavior (e.g., .c-card__body { flex: 1; } no longer stretches and meta/footer content stops pinning to the bottom). Drop the override so clickable variants keep the intended structure.
.c-card--clickable {
cursor: pointer;
text-decoration: none;
- display: block;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .c-card--clickable { | |
| cursor: pointer; | |
| text-decoration: none; | |
| display: block; | |
| } | |
| .c-card--clickable { | |
| cursor: pointer; | |
| text-decoration: none; | |
| } |
🤖 Prompt for AI Agents
In themes/beaver/assets/css/components/blocks/c-card.css around lines 205 to
209, the .c-card--clickable rule sets display:block which overrides the base
.c-card display:flex and breaks flex layout (so .c-card__body flex:1 and
footer/meta pinning stop working); remove the display:block declaration from
.c-card--clickable (keep cursor:pointer and text-decoration:none) so clickable
cards inherit the base flex layout and maintain intended stretching and
alignment.
Summary by CodeRabbit
New Features
Performance
Style
Documentation