refact: some cleanups and components renames #253
Conversation
…ctor at line 1904
…upport - Convert .fl-form-field to dual-class .c-form-field, .fl-form-field - Maintain 100% backward compatibility - Part of systematic FL Builder CSS migration
…upport - Convert input.fl-form-error to dual-class with c-form-error - Extends c-form-field coverage to input error states - Maintains 100% backward compatibility
- Convert .fl-widget to dual-class .c-widget, .fl-widget - Enable modern widget component styling - Maintains 100% backward compatibility
…pport - Convert .fl-animation to dual-class .c-animation, .fl-animation - Enable modern animation component styling - Maintains 100% backward compatibility
…port - Convert complex animation selectors to include dual-class patterns - Add c-animated class for modern animation states - Maintains 100% backward compatibility
…s support - Convert standalone .fl-animated to dual-class pattern - Complete animation-fill-mode properties dual-class support - Maintains 100% backward compatibility
…lass pattern - Add c-slideshow selector alongside fl-slideshow - Include universal child selectors (c-slideshow *, fl-slideshow *) - Maintains box-sizing: content-box for slideshow components - Zero visual impact - maintains backward compatibility
…e selector - Add c-slideshow .c-slideshow-image img alongside fl-slideshow .fl-slideshow-image img - Maintains max-width: none !important for slideshow images - Preserves nested selector functionality for both class prefixes - Zero visual impact - maintains backward compatibility
… dual-class pattern - Add c-slideshow-social selector alongside fl-slideshow-social - Maintains line-height: 0 !important for social elements - Preserves important styling for social media integration - Zero visual impact - maintains backward compatibility
…versal selector - Add c-slideshow-social * alongside fl-slideshow-social * - Maintains margin: 0 !important for all child elements - Completes slideshow social element styling migration - Zero visual impact - maintains backward compatibility
…ts to dual-class - Add c-col:before, c-col:after alongside fl-col:before, fl-col:after - Maintains clearfix functionality for grid layout system - Preserves display: table and content properties for pseudo-elements - High-frequency class (47 occurrences) - critical grid structure
…er conversion - Add c-col:after alongside fl-col:after in clearfix rule - Maintains clear: both functionality for grid column structure - Completes dual-class support for fl-col clearfix system - Essential for grid layout float clearing mechanism
…ts to dual-class - Add c-row:before, c-row:after alongside fl-row:before, fl-row:after - Maintains clearfix functionality for row layout system - Preserves display: table and content properties for pseudo-elements - High-frequency class (31 occurrences) - critical grid structure
…er conversion - Add c-row:after alongside fl-row:after in clearfix rule - Maintains clear: both functionality for grid row structure - Completes dual-class support for fl-row clearfix system - Essential for grid layout float clearing mechanism
WalkthroughAdds FAQ metadata to several blog posts, updates services description, introduces a comprehensive technical debt report, expands CSS utilities and class prefixes, and modifies Hugo layouts to use cached partials and remove inline CSS/title overrides. Adds extensive unit tests for base, home, list, single, and 404 templates covering structure, SEO, accessibility, security, and performance. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant H as Hugo Renderer
participant L as Layout (baseof/home/404/...)
participant P as PartialCached("assets/css-processor.html")
participant C as Cache
U->>H: Build site
H->>L: Render page template
alt CSS bundle required
L->>C: Lookup cache by key (e.g., "homepage"/"404"/"navigation")
alt Cache hit
C-->>L: Return bundled CSS
else Cache miss
L->>P: Invoke partialCached with resources+bundleName
P->>P: Process CSS bundle
P->>C: Store bundle under key
C-->>L: Return bundled CSS
end
end
opt Mermaid used
L->>L: Insert <script> with integrity+crossorigin
end
L-->>H: Emit final HTML
H-->>U: Serve generated page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
themes/beaver/assets/css/accessibility-focus.css (1)
103-118: Fix contrast of focused skip link (fails WCAG AA for normal text).
.skip-link:focus-visiblesets background to#1a8cffwith white text (~3.4:1 contrast). Use a darker background (or keep black) to meet 4.5:1.Apply this diff:
.skip-link:focus-visible { position: static; width: auto; height: auto; margin: 0; overflow: visible; clip: unset; outline: 3px solid #1a8cff; outline-offset: 2px; - background: #1a8cff; - color: #fff; + background: #000; + color: #fff; padding: 8px 16px; text-decoration: none; z-index: 100000; }
🧹 Nitpick comments (27)
docs/comprehensive-technical-debt-report.md (1)
4-4: Replace emphasis with a proper headingMarkdownlint (MD036) flags the emphasized line acting as a heading. Convert it to an actual heading to satisfy lint and keep structure consistent.
-*Technical Debt Report Synthesizer - September 26, 2025* +### Technical Debt Report Synthesizer - September 26, 2025content/services/_index.md (1)
3-3: Tighten meta description to <=160 chars for SERP truncation preventionCurrent copy risks truncation. Consider a concise rewrite.
-description: Transform your technology with expert engineering leadership. Ruby on Rails development, CTO consulting, team scaling. 95% client success rate. Get quote. +description: Transform your technology with expert engineering leadership: Rails development, CTO consulting, and team scaling. 95% client success rate. Get a quote.content/blog/mastering-ruby-on-rails-best-practices-for-efficient-development-in-2024/index.md (1)
250-275: Avoid duplicate FAQs in body and front‑matterThis page has an in-body “Frequently Asked Questions” section and new front‑matter
faqs. Risk: content drift or duplicate UI if templates render both.
- Option A: Keep body section; remove/trim front‑matter FAQs (or vice versa).
- Option B: Render JSON‑LD from front‑matter only; keep body as canonical source. Ensure answers match.
themes/beaver/assets/css/accessibility-focus.css (2)
28-35: Harden sr-only utility for cross‑browser consistency.Add standard properties (margin, border, padding reset; white-space) used in robust sr-only patterns.
Apply this diff:
.sr-only { position: absolute !important; clip: rect(1px, 1px, 1px, 1px); clip-path: inset(50%); width: 1px; height: 1px; overflow: hidden; + margin: -1px; + border: 0; + padding: 0; + white-space: nowrap; }Also applies to: 37-44
12-13: Avoid extreme z-index values.
z-index: 999999/100000can cause stacking wars. Prefer a documented layer scale (e.g., 1000–1100) consistent with the theme.Also applies to: 117-118
test/unit/single_template_test.rb (1)
10-13: Reduce RuboCop noise: trailing commas, complexity, and unused locals.
- Add trailing commas in multiline arrays.
- Extract helper assertions to reduce ABC/complexity in long tests (OG tags, structured data, accessibility).
- Remove unused locals (skip_links, srcset, sizes, loading, total_external, has_security_attr).
Also applies to: 58-64, 86-93, 112-119, 137-143, 150-184, 274-314
test/unit/home_template_test.rb (4)
299-302: Enforce HTTPS for external analytics scripts.Prefer HTTPS to avoid mixed content.
Apply this diff:
- assert script["src"].start_with?("http"), - "Analytics scripts should use proper URLs" + assert script["src"].start_with?("https://"), + "Analytics scripts should use HTTPS URLs"
203-208: Add trailing comma in multiline array (RuboCop).Keeps diffs cleaner and satisfies Style/TrailingCommaInArrayLiteral.
Apply this diff:
cta_indicators = [ doc.css(".cta, .call-to-action").any?, doc.css("button").any?, doc.css("a.btn, a.button").any?, - doc.css("input[type='submit']").any? + doc.css("input[type='submit']").any?, ]
214-219: Include theme button classes in CTA checks.Beaver/FL Builder often uses
.fl-button.Apply this diff:
- buttons = doc.css("button, .btn, .button") + buttons = doc.css("button, .btn, .button, .fl-button")
117-121: Remove unused locals (RuboCop Lint/UselessAssignment).
target,lazy_loading_present,responsive_classes,skip_linksare unused. Drop them to keep tests clean.Also applies to: 146-150, 233-235, 330-332
test/unit/list_template_test.rb (5)
10-14: Add trailing comma in multiline array (RuboCop).Prevents noisy diffs and satisfies Style/TrailingCommaInArrayLiteral.
Apply this diff:
@test_pages = [ "blog/index.html", "categories/index.html", - "tags/index.html" + "tags/index.html", ].select { |page| File.exist?("#{root_path}/#{page}") }
58-65: Avoid assigning and not using list indicator array.Inline the predicate into the assert to remove the unused variable.
Apply this diff:
- list_indicators = [ - doc.css("article").any?, - doc.css(".post, .post-item").any?, - doc.css(".entry, .entry-item").any?, - doc.css(".blog-post").any?, - doc.css("ul li, ol li").any? - ] - - assert list_indicators.any?, + assert( + doc.css("article, .post, .post-item, .entry, .blog-post, ul li, ol li").any?, "List page should contain a list of items (articles, posts, or list elements)" - ) + )
85-93: Add trailing comma in multiline array (RuboCop).Apply this diff:
content_indicators = [ item.css("p").any?, item.css(".excerpt, .summary, .content").any?, - item.text.strip.length > 50 + item.text.strip.length > 50, ]
212-248: Factor schema JSON-LD parsing into a helper to reduce complexity.
test_list_page_structured_data_blogis flagged for ABC/cyclomatic complexity. Extract parsing/validation helpers in BasePageTestCase.
275-281: Remove unused locals (RuboCop Lint/UselessAssignment).
has_hierarchy,placeholder,loading,total_externalare unused. Clean them up.Also applies to: 313-315, 466-468, 475-477
test/unit/baseof_template_test.rb (2)
333-339: Remove unused header/footer locals.
header_element/footer_elementare assigned but not used. Delete or reference them.Apply this diff:
- header_element = doc.css("header").first || doc.css(".header").first - # Header is optional but if present, should have proper structure - - # Should have footer content (from footer block or partial) - footer_element = doc.css("footer").first || doc.css(".footer").first - # Footer is optional but if present, should have proper structure + # Header and footer are optional but if present, should have proper structure + doc.css("header, .header, footer, .footer")
360-361: Add trailing comma in multiline array (RuboCop).Apply this diff:
valid_policies = ["no-referrer", "no-referrer-when-downgrade", "origin", "origin-when-cross-origin", "same-origin", "strict-origin", - "strict-origin-when-cross-origin", "unsafe-url"] + "strict-origin-when-cross-origin", "unsafe-url",]themes/beaver/layouts/page/contact-us.html (1)
14-14: Use a richer cache key and pass page context to the CSS partial.
- Add "context" to the dict so the partial can access site/page data if it relies on it.
- Include language (and optionally permalink) in the cache key to avoid cross-language cache collisions.
Apply this diff:
-{{ partialCached "assets/css-processor.html" (dict "resources" $contactUsResources "bundleName" "contact-us") "contact-us" }} +{{ partialCached "assets/css-processor.html" (dict "resources" $contactUsResources "bundleName" "contact-us" "context" .) (printf "contact-us-%s" .Site.Language.Lang) }}themes/beaver/layouts/page/single.html (1)
14-14: Add context and avoid cache collisions by including language in the key.Keeps behavior consistent and safe for multi-language.
-{{ partialCached "assets/css-processor.html" (dict "resources" $pageSingleResources "bundleName" "privacy-policy") "privacy-policy" }} +{{ partialCached "assets/css-processor.html" (dict "resources" $pageSingleResources "bundleName" "privacy-policy" "context" .) (printf "privacy-policy-%s" .Site.Language.Lang) }}themes/beaver/layouts/page/about.html (1)
13-13: Good switch to partialCached and passing context; refine cache key.Add language to the key to avoid multi-language cache overlap.
-{{ partialCached "assets/css-processor.html" (dict "resources" $aboutResources "bundleName" "about-us" "context" .) "about-us" }} +{{ partialCached "assets/css-processor.html" (dict "resources" $aboutResources "bundleName" "about-us" "context" .) (printf "about-us-%s" .Site.Language.Lang) }}themes/beaver/layouts/404.html (1)
13-13: Pass context and make the cache key language-aware (for multilingual sites).404 can differ per language; include context and language in the key.
-{{ partialCached "assets/css-processor.html" (dict "resources" $cssResources "bundleName" "404") "404" }} +{{ partialCached "assets/css-processor.html" (dict "resources" $cssResources "bundleName" "404" "context" .) (printf "404-%s" .Site.Language.Lang) }}themes/beaver/layouts/home.html (1)
22-22: Include context and a robust cache key for homepage CSS bundle.Consistent with other templates and safer for i18n.
-{{ partialCached "assets/css-processor.html" (dict "resources" $nonCriticalResources "bundleName" "homepage") "homepage" }} +{{ partialCached "assets/css-processor.html" (dict "resources" $nonCriticalResources "bundleName" "homepage" "context" .) (printf "homepage-%s" .Site.Language.Lang) }}themes/beaver/assets/css/beaver-grid-layout.css (1)
533-535: Mirror .fl-photo with .c-photo across dependent selectors.You added .c-photo for the base block. Ensure any downstream selectors (e.g., caption, align modifiers) that rely on .fl-photo also include .c-photo, otherwise styles may diverge.
Example:
-.fl-photo-caption { ... } +.fl-photo-caption, .c-photo-caption { ... }Review adjacent selectors for parity. Based on learnings
themes/beaver/layouts/careers/single.html (1)
15-15: Add context and language-specific cache key to CSS partialCached call
In themes/beaver/layouts/careers/single.html line 15:- {{ partialCached "assets/css-processor.html" (dict "resources" $careersResources "bundleName" "single-careers") "single-careers" }} + {{ partialCached "assets/css-processor.html" (dict "resources" $careersResources "bundleName" "single-careers" "context" .) (printf "single-careers-%s" .Site.Language.Lang) }}themes/beaver/layouts/baseof.html (1)
55-57: Mark Mermaid script as non-blocking and verify SRI
- Add
deferto the<script>tag to avoid blocking HTML parsing.- Manually verify that the
integrityhash matches the exact CDN asset by running:curl -sSL https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.min.js \ | openssl dgst -sha384 -binary | openssl base64 -A | sed 's/^/sha384-/'- <script src="https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.min.js" - integrity="sha384-o+g/BxPwhi0C3RK7oQBxQuNimeafQ3GE/ST4iT2BxVI4Wzt60SH4pq9iXVYujjaS" - crossorigin="anonymous"></script> + <script defer src="https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.min.js" + integrity="sha384-o+g/BxPwhi0C3RK7oQBxQuNimeafQ3GE/ST4iT2BxVI4Wzt60SH4pq9iXVYujjaS" + crossorigin="anonymous"></script>themes/beaver/assets/css/theme-main.css (2)
12-17: Make logo sizing responsive to preserve aspect ratioFixed width+height risks distortion and poor scaling on HiDPI. Prefer constraining one dimension and letting the other auto, plus max-width.
Apply, for example:
-.logo-image-main { - width: 200px; - height: 36px; - display: inline-block; -} +.logo-image-main { + height: 36px; + width: auto; + max-width: 100%; + display: inline-block; +}
1901-1906: c-fixed-width parity likely incomplete across fixed-width rulesAdding body.c-fixed-width here is good, but other fixed-width media blocks (e.g., widths for .fl-fixed-width .fl-page-header-fixed at Lines 2691-2704) don’t have c-variants, which may cause layout divergence.
Audit and mirror .fl-fixed-width and .fl-page fixed-width rules with .c-fixed-width where applicable for consistent behavior.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
content/blog/best-practices-for-optimizing-ruby-on-rails-performance/index.md(1 hunks)content/blog/deploying-ruby-on-rails-applications-with-kamal-devops-docker/index.md(1 hunks)content/blog/mastering-ruby-on-rails-best-practices-for-efficient-development-in-2024/index.md(1 hunks)content/blog/rails-8-introducing-new-default-asset-pipeline-propshaft-ruby/index.md(1 hunks)content/services/_index.md(1 hunks)docs/comprehensive-technical-debt-report.md(1 hunks)test/unit/404_template_test.rb(1 hunks)test/unit/baseof_template_test.rb(1 hunks)test/unit/home_template_test.rb(1 hunks)test/unit/list_template_test.rb(1 hunks)test/unit/single_template_test.rb(1 hunks)themes/beaver/assets/css/accessibility-focus.css(1 hunks)themes/beaver/assets/css/beaver-grid-layout.css(6 hunks)themes/beaver/assets/css/fl-homepage-layout.css(1 hunks)themes/beaver/assets/css/theme-main.css(5 hunks)themes/beaver/layouts/404.html(1 hunks)themes/beaver/layouts/baseof.html(2 hunks)themes/beaver/layouts/careers/single.html(1 hunks)themes/beaver/layouts/clients/single.html(1 hunks)themes/beaver/layouts/home.html(1 hunks)themes/beaver/layouts/page/about.html(1 hunks)themes/beaver/layouts/page/contact-us.html(1 hunks)themes/beaver/layouts/page/single.html(1 hunks)themes/beaver/layouts/single.html(0 hunks)
💤 Files with no reviewable changes (1)
- themes/beaver/layouts/single.html
🧰 Additional context used
📓 Path-based instructions (5)
{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/clients/single.htmlthemes/beaver/layouts/careers/single.htmlthemes/beaver/layouts/page/contact-us.htmlthemes/beaver/layouts/page/single.htmlthemes/beaver/layouts/home.htmlthemes/beaver/layouts/404.htmlthemes/beaver/layouts/page/about.htmlthemes/beaver/layouts/baseof.html
test/**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Ruby tests must be named using *_test.rb naming convention
Files:
test/unit/404_template_test.rbtest/unit/single_template_test.rbtest/unit/baseof_template_test.rbtest/unit/list_template_test.rbtest/unit/home_template_test.rb
test/**/*_test.rb
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*_test.rb: Tests must include assertions (assert_/refute_/flunk) and validate behavior, not output
Prohibit output statements in tests (puts, print, p) that mask failures
Files:
test/unit/404_template_test.rbtest/unit/single_template_test.rbtest/unit/baseof_template_test.rbtest/unit/list_template_test.rbtest/unit/home_template_test.rb
**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Use ENV.fetch for configurable values (e.g., ports, paths); avoid hardcoded configuration constants
Files:
test/unit/404_template_test.rbtest/unit/single_template_test.rbtest/unit/baseof_template_test.rbtest/unit/list_template_test.rbtest/unit/home_template_test.rb
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/beaver-grid-layout.cssthemes/beaver/assets/css/accessibility-focus.cssthemes/beaver/assets/css/fl-homepage-layout.cssthemes/beaver/assets/css/theme-main.css
🧠 Learnings (1)
📚 Learning: 2024-11-08T06:11:26.144Z
Learnt from: AucT
PR: jetthoughts/jetthoughts.github.io#200
File: themes/beaver/assets/js/tabs.js:17-46
Timestamp: 2024-11-08T06:11:26.144Z
Learning: For the file `themes/beaver/assets/js/tabs.js`, avoid suggesting improvements to keyboard navigation implementation or adding ARIA roles and attributes unless requested.
Applied to files:
themes/beaver/assets/css/accessibility-focus.css
🧬 Code graph analysis (11)
themes/beaver/layouts/clients/single.html (2)
test/unit/hugo_partials_test.rb (1)
test_asset_processing_partials(85-108)test/unit/template_cleanup_validation_test.rb (1)
test_template_asset_processing(96-125)
themes/beaver/layouts/careers/single.html (2)
test/unit/hugo_partials_test.rb (1)
test_asset_processing_partials(85-108)test/unit/template_cleanup_validation_test.rb (1)
test_template_asset_processing(96-125)
themes/beaver/layouts/page/contact-us.html (2)
test/unit/hugo_partials_test.rb (2)
test_asset_processing_partials(85-108)test_page_header_partial_with_css(18-39)test/unit/template_cleanup_validation_test.rb (1)
test_template_asset_processing(96-125)
test/unit/404_template_test.rb (2)
test/unit/template_cleanup_validation_test.rb (3)
test_404_page_structure(80-94)setup(3-330)setup(7-22)test/system/desktop_site_test.rb (1)
test_not_found(180-184)
test/unit/single_template_test.rb (2)
test/unit/list_template_test.rb (2)
setup(3-478)setup(8-18)test/base_page_test_case.rb (8)
root_path(12-14)parse_html_file(16-27)assert_valid_url(57-59)extract_json_ld_schemas(29-31)assert_schema_context(46-48)assert_schema_fields(50-54)assert_valid_date(61-64)private(9-91)
themes/beaver/layouts/page/single.html (2)
test/unit/hugo_partials_test.rb (1)
test_asset_processing_partials(85-108)test/unit/template_cleanup_validation_test.rb (1)
test_template_asset_processing(96-125)
test/unit/baseof_template_test.rb (2)
test/base_page_test_case.rb (2)
parse_html_file(16-27)private(9-91)test/unit/template_cleanup_validation_test.rb (1)
test_homepage_renders_correctly(24-47)
themes/beaver/layouts/home.html (2)
test/unit/hugo_partials_test.rb (3)
test_asset_processing_partials(85-108)test_performance_partial_integration(286-316)test_page_header_partial_with_css(18-39)test/unit/template_cleanup_validation_test.rb (1)
test_template_asset_processing(96-125)
test/unit/list_template_test.rb (1)
test/base_page_test_case.rb (5)
root_path(12-14)parse_html_file(16-27)extract_json_ld_schemas(29-31)assert_schema_context(46-48)assert_schema_fields(50-54)
test/unit/home_template_test.rb (2)
test/base_page_test_case.rb (5)
parse_html_file(16-27)extract_json_ld_schemas(29-31)assert_schema_context(46-48)assert_schema_fields(50-54)assert_valid_url(57-59)test/unit/template_cleanup_validation_test.rb (1)
test_homepage_renders_correctly(24-47)
themes/beaver/layouts/page/about.html (2)
test/unit/hugo_partials_test.rb (2)
test_asset_processing_partials(85-108)test_critical_css_partial(168-185)test/unit/template_cleanup_validation_test.rb (1)
test_template_asset_processing(96-125)
🪛 RuboCop (1.80.2)
test/unit/404_template_test.rb
[convention] 57-95: Assignment Branch Condition size for test_404_page_provides_helpful_navigation is too high. [<9, 33, 9> 35.37/23]
(Metrics/AbcSize)
[convention] 66-66: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[convention] 105-105: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[convention] 125-156: Assignment Branch Condition size for test_404_page_suggests_popular_content is too high. [<7, 27, 6> 28.53/23]
(Metrics/AbcSize)
[convention] 134-134: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[convention] 249-249: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[convention] 256-296: Assignment Branch Condition size for test_404_page_accessibility_features is too high. [<15, 24, 7> 29.15/23]
(Metrics/AbcSize)
[warning] 264-264: Useless assignment to variable - skip_links.
(Lint/UselessAssignment)
[convention] 298-334: Assignment Branch Condition size for test_404_page_user_experience_elements is too high. [<7, 23, 1> 24.06/23]
(Metrics/AbcSize)
[warning] 306-306: Useless assignment to variable - has_technical_terms.
(Lint/UselessAssignment)
[convention] 319-319: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[warning] 326-326: Useless assignment to variable - apologetic_indicators.
(Lint/UselessAssignment)
[convention] 329-329: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[convention] 336-362: Assignment Branch Condition size for test_404_page_contact_information is too high. [<7, 22, 3> 23.28/23]
(Metrics/AbcSize)
[convention] 345-345: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[warning] 373-373: Useless assignment to variable - total_external.
(Lint/UselessAssignment)
[warning] 422-422: Useless assignment to variable - rel.
(Lint/UselessAssignment)
test/unit/single_template_test.rb
[convention] 12-12: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[convention] 96-127: Assignment Branch Condition size for test_single_page_open_graph_tags is too high. [<7, 25, 6> 26.65/23]
(Metrics/AbcSize)
[convention] 150-184: Assignment Branch Condition size for test_single_page_structured_data_article is too high. [<6, 27, 9> 29.09/23]
(Metrics/AbcSize)
[convention] 150-184: Cyclomatic complexity for test_single_page_structured_data_article is too high. [8/7]
(Metrics/CyclomaticComplexity)
[warning] 242-242: Useless assignment to variable - related_indicators.
(Lint/UselessAssignment)
[convention] 246-246: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[convention] 261-261: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[convention] 274-314: Assignment Branch Condition size for test_single_page_accessibility_features is too high. [<15, 24, 8> 29.41/23]
(Metrics/AbcSize)
[convention] 274-314: Cyclomatic complexity for test_single_page_accessibility_features is too high. [8/7]
(Metrics/CyclomaticComplexity)
[warning] 278-278: Useless assignment to variable - skip_links.
(Lint/UselessAssignment)
[warning] 325-325: Useless assignment to variable - srcset.
(Lint/UselessAssignment)
[warning] 326-326: Useless assignment to variable - sizes.
(Lint/UselessAssignment)
[warning] 333-333: Useless assignment to variable - loading.
(Lint/UselessAssignment)
[warning] 342-342: Useless assignment to variable - total_external.
(Lint/UselessAssignment)
[warning] 364-364: Useless assignment to variable - has_security_attr.
(Lint/UselessAssignment)
test/unit/baseof_template_test.rb
[warning] 333-333: Useless assignment to variable - header_element.
(Lint/UselessAssignment)
[warning] 337-337: Useless assignment to variable - footer_element.
(Lint/UselessAssignment)
[convention] 360-360: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
test/unit/list_template_test.rb
[convention] 13-13: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[warning] 31-31: Useless assignment to variable - has_list_indicator.
(Lint/UselessAssignment)
[convention] 63-63: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[convention] 70-107: Assignment Branch Condition size for test_list_page_item_structure is too high. [<9, 27, 6> 29.09/23]
(Metrics/AbcSize)
[convention] 88-88: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[warning] 123-123: Useless assignment to variable - has_list_keyword.
(Lint/UselessAssignment)
[convention] 212-248: Assignment Branch Condition size for test_list_page_structured_data_blog is too high. [<7, 26, 12> 29.48/23]
(Metrics/AbcSize)
[convention] 212-248: Cyclomatic complexity for test_list_page_structured_data_blog is too high. [9/7]
(Metrics/CyclomaticComplexity)
[convention] 212-248: Perceived complexity for test_list_page_structured_data_blog is too high. [9/8]
(Metrics/PerceivedComplexity)
[warning] 275-275: Useless assignment to variable - has_hierarchy.
(Lint/UselessAssignment)
[warning] 313-313: Useless assignment to variable - placeholder.
(Lint/UselessAssignment)
[convention] 336-340: Avoid more than 3 levels of block nesting.
(Metrics/BlockNesting)
[convention] 413-452: Assignment Branch Condition size for test_list_page_accessibility_features is too high. [<15, 25, 9> 30.51/23]
(Metrics/AbcSize)
[warning] 448-448: Useless assignment to variable - is_generic.
(Lint/UselessAssignment)
[warning] 466-466: Useless assignment to variable - loading.
(Lint/UselessAssignment)
[warning] 475-475: Useless assignment to variable - total_external.
(Lint/UselessAssignment)
test/unit/home_template_test.rb
[convention] 95-95: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[warning] 117-117: Useless assignment to variable - target.
(Lint/UselessAssignment)
[convention] 126-151: Cyclomatic complexity for test_homepage_performance_critical_elements is too high. [9/7]
(Metrics/CyclomaticComplexity)
[convention] 126-151: Perceived complexity for test_homepage_performance_critical_elements is too high. [9/8]
(Metrics/PerceivedComplexity)
[warning] 146-146: Useless assignment to variable - lazy_loading_present.
(Lint/UselessAssignment)
[convention] 207-207: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
[warning] 233-233: Useless assignment to variable - responsive_classes.
(Lint/UselessAssignment)
[convention] 282-308: Cyclomatic complexity for test_homepage_analytics_integration is too high. [10/7]
(Metrics/CyclomaticComplexity)
[convention] 282-308: Perceived complexity for test_homepage_analytics_integration is too high. [11/8]
(Metrics/PerceivedComplexity)
[warning] 330-330: Useless assignment to variable - skip_links.
(Lint/UselessAssignment)
🪛 markdownlint-cli2 (0.18.1)
docs/comprehensive-technical-debt-report.md
4-4: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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
🔇 Additional comments (12)
content/blog/mastering-ruby-on-rails-best-practices-for-efficient-development-in-2024/index.md (1)
22-36: .Params.faqs consumed by SEO and UI templates
Verified.Params.faqsis used inthemes/beaver/layouts/partials/seo/faq-schema.html(JSON-LD FAQPage) and in theservices/faqs.htmlpartial for UI rendering.content/services/_index.md (1)
8-11: Assets verified; no missing filescareers-browse-apply-preapre.jpeg,og-services.jpg, andjohn-doe.jpgall exist. Ifpreapreis a typo, rename accordingly.content/blog/rails-8-introducing-new-default-asset-pipeline-propshaft-ruby/index.md (1)
21-33: Front-matter FAQs are rendered by the FAQ schema partial
Thefaq-schema.htmlpartial consumes.Params.faqsand outputs JSON-LD FAQPage markup.content/blog/best-practices-for-optimizing-ruby-on-rails-performance/index.md (1)
22-34: FAQ metadata wired up for JSON-LDThe
seo/faq-schema.htmlpartial is included inbaseof.htmland processes.Params.faqsto output structured JSON-LD.content/blog/deploying-ruby-on-rails-applications-with-kamal-devops-docker/index.md (1)
22-36: FAQPage JSON-LD generation confirmed
Thethemes/beaver/layouts/partials/seo/faq-schema.htmlpartial correctly reads.Params.faqs(falling back to.Params.faq.questions) to emitFAQPageJSON-LD. Consider aligning the.envguidance in your post with your project’s documented environment-variable usage.test/unit/baseof_template_test.rb (1)
21-25: Version pin on Mermaid may be brittle. Verify project intent.Asserting
mermaid@11couples tests to a specific CDN/version. Confirm this is a deliberate requirement; otherwise, assert presence of version and SRI generically.Would you like me to relax this to assert SRI/crossorigin with any pinned version?
themes/beaver/assets/css/fl-homepage-layout.css (1)
3050-3054: Dual column selector alignment looks good
c-columnsharing the float/min-height rule withfl-colkeeps the legacy layout behaviour intact while enabling the new prefix. No issues spotted.themes/beaver/layouts/baseof.html (1)
27-27: Nice: partialCached with context for navigation CSS.This matches best practices and should improve build performance.
themes/beaver/assets/css/beaver-grid-layout.css (3)
9-10: c- prefix support for row/col clearfixes looks good.Adding .c-row/.c-col in clearfix rules maintains behavior parity for the new prefix.
Also applies to: 15-16, 25-26, 28-28
714-719: Form field error parity: include .c- and .fl- variants consistently.This keeps validation styles consistent across prefixes. LGTM.
735-749: Animation visibility parity is correct; watch for selector weight.Combining selectors increases matches; ensure no unintended overrides. Otherwise good.
Please sanity-check in a browser that elements with only .fl- or only .c- still animate as expected.
themes/beaver/assets/css/theme-main.css (1)
353-355: Dual-prefix support for full-width nav looks goodSelector extension to .c-full-width maintains behavior.
| puts "INFO: 404 page allows indexing - consider noindex for traditional SEO approach" | ||
| end | ||
| else | ||
| puts "INFO: No robots meta tag found - 404 pages typically benefit from noindex directive" | ||
| end | ||
|
|
||
| # Canonical should not point to 404 page itself | ||
| canonical_link = doc.css("head link[rel='canonical']").first | ||
| if canonical_link | ||
| href = canonical_link["href"] | ||
| # Note: Some 404 implementations may canonicalize to themselves for SEO reasons | ||
| if href.include?("404") | ||
| puts "INFO: 404 page canonical points to itself - consider alternative canonical strategy" | ||
| end |
There was a problem hiding this comment.
Remove stdout logging from tests.
puts-style logging inside tests violates the repo’s testing guidelines (no output statements) and pollutes test output. Please drop these statements or replace them with assertions/structured diagnostics that keep the suite quiet. Here’s a minimal edit that removes the offending calls and keeps the informational intent as comments:
@@
- if robots_meta
- robots_content = robots_meta["content"].downcase
-
- # Should prevent indexing - however, some SEO strategies allow indexing for link discovery
- indexing_prevented = robots_content.include?("noindex") ||
- robots_content.include?("none")
-
- # This is informational - some sites allow 404 indexing for SEO discovery
- unless indexing_prevented
- puts "INFO: 404 page allows indexing - consider noindex for traditional SEO approach"
- end
- else
- puts "INFO: No robots meta tag found - 404 pages typically benefit from noindex directive"
- end
+ if robots_meta
+ robots_content = robots_meta["content"].downcase
+
+ # Should prevent indexing - however, some SEO strategies allow indexing for link discovery
+ indexing_prevented = robots_content.include?("noindex") ||
+ robots_content.include?("none")
+ end
@@
- if canonical_link
- href = canonical_link["href"]
- # Note: Some 404 implementations may canonicalize to themselves for SEO reasons
- if href.include?("404")
- puts "INFO: 404 page canonical points to itself - consider alternative canonical strategy"
- end
- end
+ if canonical_link
+ href = canonical_link["href"]
+ # Note: Some 404 implementations may canonicalize to themselves for SEO reasons
+ # Consider avoiding a canonical that points directly to the 404 URL.
+ endAs per coding guidelines.
📝 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.
| puts "INFO: 404 page allows indexing - consider noindex for traditional SEO approach" | |
| end | |
| else | |
| puts "INFO: No robots meta tag found - 404 pages typically benefit from noindex directive" | |
| end | |
| # Canonical should not point to 404 page itself | |
| canonical_link = doc.css("head link[rel='canonical']").first | |
| if canonical_link | |
| href = canonical_link["href"] | |
| # Note: Some 404 implementations may canonicalize to themselves for SEO reasons | |
| if href.include?("404") | |
| puts "INFO: 404 page canonical points to itself - consider alternative canonical strategy" | |
| end | |
| if robots_meta | |
| robots_content = robots_meta["content"].downcase | |
| # Should prevent indexing - however, some SEO strategies allow indexing for link discovery | |
| indexing_prevented = robots_content.include?("noindex") || | |
| robots_content.include?("none") | |
| end | |
| # Canonical should not point to 404 page itself | |
| canonical_link = doc.css("head link[rel='canonical']").first | |
| if canonical_link | |
| href = canonical_link["href"] | |
| # Note: Some 404 implementations may canonicalize to themselves for SEO reasons | |
| # Consider avoiding a canonical that points directly to the 404 URL. | |
| end |
🤖 Prompt for AI Agents
In test/unit/404_template_test.rb around lines 197 to 210, remove the puts calls
that emit INFO lines to stdout; replace them with either comments preserving the
informational text or with appropriate assertions (e.g., assert or refute) that
validate the condition silently. Specifically, delete the three puts statements
and either convert the messages into comments above the related checks or
replace them with assertions that fail only when the condition is unexpected,
keeping the test output quiet and consistent with repo guidelines.
| html_lang = doc.root["lang"] | ||
| refute_nil html_lang, "HTML element should have lang attribute for accessibility" | ||
| assert_equal "en-US", html_lang, "Language should be set to en-US" | ||
|
|
||
| # Check charset | ||
| charset_meta = doc.css("head meta[charset]").first | ||
| refute_nil charset_meta, "Document should have charset meta tag" | ||
| assert_equal "UTF-8", charset_meta["charset"], "Charset should be UTF-8" | ||
| end |
There was a problem hiding this comment.
Relax overly strict lang assertion.
Hard-coding en-US will fail for en, en-GB, or localized builds. Assert presence and English prefix.
Apply this diff:
- html_lang = doc.root["lang"]
- refute_nil html_lang, "HTML element should have lang attribute for accessibility"
- assert_equal "en-US", html_lang, "Language should be set to en-US"
+ html_lang = doc.root["lang"]
+ refute_nil html_lang, "HTML element should have lang attribute for accessibility"
+ assert html_lang.downcase.start_with?("en"),
+ "Language should start with 'en' (got #{html_lang.inspect})"📝 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.
| html_lang = doc.root["lang"] | |
| refute_nil html_lang, "HTML element should have lang attribute for accessibility" | |
| assert_equal "en-US", html_lang, "Language should be set to en-US" | |
| # Check charset | |
| charset_meta = doc.css("head meta[charset]").first | |
| refute_nil charset_meta, "Document should have charset meta tag" | |
| assert_equal "UTF-8", charset_meta["charset"], "Charset should be UTF-8" | |
| end | |
| html_lang = doc.root["lang"] | |
| refute_nil html_lang, "HTML element should have lang attribute for accessibility" | |
| assert html_lang.downcase.start_with?("en"), | |
| "Language should start with 'en' (got #{html_lang.inspect})" |
🤖 Prompt for AI Agents
In test/unit/baseof_template_test.rb around lines 129 to 137, the test currently
asserts the HTML lang equals "en-US" which is too strict; change it to assert
the lang attribute exists and that its value begins with the English prefix
(e.g., starts with "en" or matches /^en/), allowing "en", "en-US", "en-GB",
etc.; keep the charset checks intact.
| @test_pages = [ | ||
| "blog/index.html", | ||
| "about/index.html" | ||
| ].select { |page| File.exist?("#{root_path}/#{page}") } | ||
|
|
||
| skip "No single pages found for testing" if @test_pages.empty? | ||
| @test_page = @test_pages.first | ||
| end |
There was a problem hiding this comment.
Select real single pages; avoid blog list index.
Using "blog/index.html" targets a list page, not a single. This will make several assertions unreliable.
Apply this diff to pick actual singles under blog/posts and fall back to About:
- @test_pages = [
- "blog/index.html",
- "about/index.html"
- ].select { |page| File.exist?("#{root_path}/#{page}") }
+ candidates = Dir.glob("#{root_path}/{blog,posts}/**/index.html")
+ # Exclude top-level list pages like blog/index.html
+ candidates.reject! { |p| p.end_with?("blog/index.html") }
+ candidates << "#{root_path}/about/index.html"
+ @test_pages = candidates
+ .uniq
+ .select { |p| File.exist?(p) }
+ .map { |p| p.sub("#{root_path}/", "") }📝 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.
| @test_pages = [ | |
| "blog/index.html", | |
| "about/index.html" | |
| ].select { |page| File.exist?("#{root_path}/#{page}") } | |
| skip "No single pages found for testing" if @test_pages.empty? | |
| @test_page = @test_pages.first | |
| end | |
| candidates = Dir.glob("#{root_path}/{blog,posts}/**/index.html") | |
| # Exclude top-level list pages like blog/index.html | |
| candidates.reject! { |p| p.end_with?("blog/index.html") } | |
| candidates << "#{root_path}/about/index.html" | |
| @test_pages = candidates | |
| .uniq | |
| .select { |p| File.exist?(p) } | |
| .map { |p| p.sub("#{root_path}/", "") } | |
| skip "No single pages found for testing" if @test_pages.empty? | |
| @test_page = @test_pages.first | |
| end |
🧰 Tools
🪛 RuboCop (1.80.2)
[convention] 12-12: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
🤖 Prompt for AI Agents
In test/unit/single_template_test.rb around lines 10 to 17, the test currently
selects "blog/index.html" which is a list page rather than a single post; change
the selection logic to prefer actual single pages under blog/posts (e.g. any
files matching "#{root_path}/blog/posts/*.html") and only fall back to
"about/index.html" if no post files exist; update the File.exist?/select logic
to glob for blog/posts, map to relative paths, and then set @test_page =
@test_pages.first after the fallback so assertions operate on a true single
page.
| def test_single_page_related_content_navigation | ||
| doc = parse_html_file(@test_page) | ||
|
|
||
| # Check for related content or navigation aids | ||
| related_indicators = [ | ||
| doc.css(".related, .related-posts, .related-content").any?, | ||
| doc.css(".next-post, .prev-post, .post-navigation").any?, | ||
| doc.css(".tags, .categories").any?, | ||
| doc.css("nav.pagination").any? | ||
| ] | ||
|
|
||
| # Related content is optional but enhances user experience | ||
| # This is informational rather than a strict requirement | ||
| end |
There was a problem hiding this comment.
Add assertions or skip; test currently does nothing.
test_single_page_related_content_navigation has no assertions. Tests must assert or explicitly skip.
Apply this diff to assert conditionally:
def test_single_page_related_content_navigation
doc = parse_html_file(@test_page)
# Check for related content or navigation aids
- related_indicators = [
- doc.css(".related, .related-posts, .related-content").any?,
- doc.css(".next-post, .prev-post, .post-navigation").any?,
- doc.css(".tags, .categories").any?,
- doc.css("nav.pagination").any?
- ]
-
- # Related content is optional but enhances user experience
- # This is informational rather than a strict requirement
+ any_related = doc.css(".related, .related-posts, .related-content, \
+ .next-post, .prev-post, .post-navigation, \
+ .tags, .categories, nav.pagination").any?
+ # Optional: assert only when present, otherwise skip to keep test meaningful
+ skip "No related content/navigation present (optional feature)" unless any_related
+ assert any_related, "Related content/navigation should be detectable when implemented"
end🧰 Tools
🪛 RuboCop (1.80.2)
[warning] 242-242: Useless assignment to variable - related_indicators.
(Lint/UselessAssignment)
[convention] 246-246: Put a comma after the last item of a multiline array.
(Style/TrailingCommaInArrayLiteral)
🤖 Prompt for AI Agents
In test/unit/single_template_test.rb around lines 238 to 251, the
test_single_page_related_content_navigation method contains no assertions and
should either assert or explicitly skip; update the method to compute
related_indicators as currently written and then add a conditional: if
related_indicators.any? assert true (or assert related_indicators.any?,
"expected at least one related/navigation element") else skip "No
related/navigation elements present; related content is optional" so the test
either verifies presence or documents intentional skip.
| def test_single_page_security_considerations | ||
| doc = parse_html_file(@test_page) | ||
|
|
||
| # External links should have proper security attributes | ||
| external_links = doc.css("a[href^='http']").reject do |link| | ||
| href = link["href"] | ||
| href.include?("jetthoughts.com") || href.include?(request_domain) | ||
| end | ||
|
|
||
| external_links.each do |link| | ||
| rel = link["rel"] | ||
|
|
||
| # External links benefit from security attributes | ||
| # This is a recommendation for security best practices | ||
| if rel | ||
| security_keywords = ["noopener", "noreferrer", "nofollow"] | ||
| has_security_attr = security_keywords.any? { |keyword| rel.include?(keyword) } | ||
|
|
||
| # Security attributes are recommended but not strictly required | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Add meaningful security assertions or skip.
The test computes values but never asserts. Ensure at least one check (e.g., rel attributes on external links) or skip when none exist.
Apply this diff:
def test_single_page_security_considerations
doc = parse_html_file(@test_page)
# External links should have proper security attributes
external_links = doc.css("a[href^='http']").reject do |link|
href = link["href"]
href.include?("jetthoughts.com") || href.include?(request_domain)
end
- external_links.each do |link|
- rel = link["rel"]
-
- # External links benefit from security attributes
- # This is a recommendation for security best practices
- if rel
- security_keywords = ["noopener", "noreferrer", "nofollow"]
- has_security_attr = security_keywords.any? { |keyword| rel.include?(keyword) }
-
- # Security attributes are recommended but not strictly required
- end
- end
+ if external_links.any?
+ missing_security = external_links.reject do |link|
+ rel = (link["rel"] || "").downcase
+ rel.include?("noopener") || rel.include?("noreferrer")
+ end
+ assert missing_security.empty?, "External links should set rel='noopener' or 'noreferrer'"
+ else
+ skip "No external links found (nothing to validate)"
+ end
end📝 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.
| def test_single_page_security_considerations | |
| doc = parse_html_file(@test_page) | |
| # External links should have proper security attributes | |
| external_links = doc.css("a[href^='http']").reject do |link| | |
| href = link["href"] | |
| href.include?("jetthoughts.com") || href.include?(request_domain) | |
| end | |
| external_links.each do |link| | |
| rel = link["rel"] | |
| # External links benefit from security attributes | |
| # This is a recommendation for security best practices | |
| if rel | |
| security_keywords = ["noopener", "noreferrer", "nofollow"] | |
| has_security_attr = security_keywords.any? { |keyword| rel.include?(keyword) } | |
| # Security attributes are recommended but not strictly required | |
| end | |
| end | |
| end | |
| def test_single_page_security_considerations | |
| doc = parse_html_file(@test_page) | |
| # External links should have proper security attributes | |
| external_links = doc.css("a[href^='http']").reject do |link| | |
| href = link["href"] | |
| href.include?("jetthoughts.com") || href.include?(request_domain) | |
| end | |
| if external_links.any? | |
| missing_security = external_links.reject do |link| | |
| rel = (link["rel"] || "").downcase | |
| rel.include?("noopener") || rel.include?("noreferrer") | |
| end | |
| assert missing_security.empty?, "External links should set rel='noopener' or 'noreferrer'" | |
| else | |
| skip "No external links found (nothing to validate)" | |
| end | |
| end |
🧰 Tools
🪛 RuboCop (1.80.2)
[warning] 364-364: Useless assignment to variable - has_security_attr.
(Lint/UselessAssignment)
🤖 Prompt for AI Agents
In test/unit/single_template_test.rb around lines 348 to 369 the test computes
external_links and inspects rel but contains no assertions; either assert the
expected security attributes on each external link or explicitly skip the test
when there are no external links. Modify the test so it first returns skip "no
external links to check" if external_links.empty?, otherwise iterate
external_links and assert that link["rel"] is present and includes at least one
of "noopener" or "noreferrer" (e.g., assert rel, "external link missing rel" and
assert(rel.include?("noopener") || rel.include?("noreferrer"), "external link
rel must include noopener or noreferrer")), ensuring the test fails when
security attributes are absent.
| .c-slideshow, .fl-slideshow, | ||
| .c-slideshow *, .fl-slideshow * { | ||
| -webkit-box-sizing: content-box; | ||
| -moz-box-sizing: content-box; | ||
| box-sizing: content-box; | ||
| } | ||
|
|
||
| .fl-slideshow .fl-slideshow-image img { | ||
| .c-slideshow .c-slideshow-image img, .fl-slideshow .fl-slideshow-image img { | ||
| max-width: none !important; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Slideshow parity: Good addition of .c-slideshow variants.
Extending to .c-slideshow and its children maintains feature parity.
Scan for any remaining slideshow selectors tied only to .fl-*:
Also applies to: 650-656
🏁 Script executed:
#!/bin/bash
rg -n -C1 '\.fl-slideshow' themes/beaver/assets/css | sed -n '1,120p'Length of output: 6838
Add .c-slideshow variants to all .fl-slideshow selector blocks in layout CSS files
Include .c-slideshow, .c-slideshow *, .c-slideshow .c-slideshow-image img, .c-slideshow-social, and .c-slideshow-social * alongside their .fl-slideshow counterparts in every themes/beaver/assets/css/*-layout.css file.
🤖 Prompt for AI Agents
In themes/beaver/assets/css/beaver-grid-layout.css around lines 639 to 647, the
rules target .fl-slideshow variants but omit the equivalent .c-slideshow
selectors; update the selector lists so each .fl-slideshow block also includes
.c-slideshow and the corresponding nested variants (.c-slideshow *, .c-slideshow
.c-slideshow-image img, .c-slideshow-social, .c-slideshow-social *) where
applicable, and apply the same change across all
themes/beaver/assets/css/*-layout.css files so every .fl-slideshow selector
block has the matching .c-slideshow counterparts.
| .c-widget, .fl-widget { | ||
| margin-bottom: 40px; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Extending only the container (.c-widget) creates inconsistent widget styling
Descendant rules still target .fl-widget only (titles, lists, sub-menus). .c-widget instances will miss those resets and spacing.
Expand the related selectors to include .c-widget equivalents. Example adjustments:
-.fl-widget .fl-widget-title { ...
+.c-widget .fl-widget-title,
+.fl-widget .fl-widget-title { ...-.fl-widget ul { ...
+.c-widget ul,
+.fl-widget ul { ...-.fl-widget ul li { ...
+.c-widget ul li,
+.fl-widget ul li { ...-.fl-widget ul li:last-child { ...
+.c-widget ul li:last-child,
+.fl-widget ul li:last-child { ...-.fl-widget ul.sub-menu li { ...
+.c-widget ul.sub-menu li,
+.fl-widget ul.sub-menu li { ...-.fl-widget ul.sub-menu li:before { ...
+.c-widget ul.sub-menu li:before,
+.fl-widget ul.sub-menu li:before { ...To check coverage of .c-widget styles across the repo:
🏁 Script executed:
#!/bin/bash
# Find all CSS rules referencing .c-widget and compare against .fl-widget
rg -n --type=css -C1 '\.c-widget|\.fl-widget'Length of output: 1478
Extend all .fl-widget descendant selectors to also target .c-widget
In theme-main.css (lines 1097–1121), update each rule scoped to .fl-widget descendants (e.g. .fl-widget .fl-widget-title, .fl-widget ul, .fl-widget ul li:last-child, .fl-widget ul.sub-menu li:before, etc.) to include the equivalent .c-widget selector so .c-widget instances inherit the same title, list, and spacing resets.
| .fl-page button:visited, | ||
| .fl-responsive-preview-content button:visited, | ||
| .c-responsive-preview-content button:visited, | ||
| .fl-button-lightbox-content button:visited, | ||
| .fl-page input[type="button"], | ||
| .fl-responsive-preview-content input[type="button"], | ||
| .c-responsive-preview-content input[type="button"], | ||
| .fl-button-lightbox-content input[type="button"], | ||
| .fl-page input[type="submit"], | ||
| .fl-responsive-preview-content input[type="submit"], | ||
| .c-responsive-preview-content input[type="submit"], | ||
| .fl-button-lightbox-content input[type="submit"], | ||
| .fl-page a.fl-button, | ||
| .fl-responsive-preview-content a.fl-button, | ||
| .c-responsive-preview-content a.fl-button, | ||
| .fl-button-lightbox-content a.fl-button, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Incomplete .c-responsive-preview-content coverage across related groups
You added .c-responsive-preview-content to the base visited/inputs/buttons group, but the subsequent “children (*)” and “:hover” groups (Lines 3171-3205, 3208-3238, 3240-3267) don’t include the c-variant, breaking parity in previews.
Add .c-responsive-preview-content equivalents to the sibling groups. Examples:
-.fl-page input[type="button"] *,
-.fl-responsive-preview-content input[type="button"] *,
+.fl-page input[type="button"] *,
+.fl-responsive-preview-content input[type="button"] *,
+.c-responsive-preview-content input[type="button"] *,-.fl-page a.fl-button:hover,
-.fl-responsive-preview-content a.fl-button:hover,
+.fl-page a.fl-button:hover,
+.fl-responsive-preview-content a.fl-button:hover,
+.c-responsive-preview-content a.fl-button:hover,-.fl-page a.fl-button:hover *,
-.fl-responsive-preview-content a.fl-button:hover *,
+.fl-page a.fl-button:hover *,
+.fl-responsive-preview-content a.fl-button:hover *,
+.c-responsive-preview-content a.fl-button:hover *,Run to spot missing c-variants near existing fl-responsive selectors:
🏁 Script executed:
#!/bin/bash
# Show groups where .fl-responsive-preview-content appears without .c-responsive-preview-content
rg -n -C2 --type=css '\.fl-responsive-preview-content(?!.*\.c-responsive-preview-content)'Length of output: 155
🏁 Script executed:
#!/bin/bash
file="themes/beaver/assets/css/theme-main.css"
for range in "3171 3205" "3208 3238" "3240 3267"; do
start=${range% *}
end=${range#* }
echo "Checking lines $start–$end:"
if sed -n "${start},${end}p" "$file" | grep -q '\.c-responsive-preview-content'; then
echo " ✓ .c-responsive-preview-content present"
else
echo " ✗ .c-responsive-preview-content missing"
fi
doneLength of output: 203
Add .c-responsive-preview-content to child (*) and :hover selector groups
In themes/beaver/assets/css/theme-main.css at lines 3171–3205, 3208–3238 and 3240–3267 the child and hover blocks omit .c-responsive-preview-content, causing inconsistent styling. Example diffs:
-.fl-page input[type="button"] *,
-.fl-responsive-preview-content input[type="button"] *,
+.fl-page input[type="button"] *,
+.fl-responsive-preview-content input[type="button"] *,
+.c-responsive-preview-content input[type="button"] *,-.fl-page a.fl-button:hover,
-.fl-responsive-preview-content a.fl-button:hover,
+.fl-page a.fl-button:hover,
+.fl-responsive-preview-content a.fl-button:hover,
+.c-responsive-preview-content a.fl-button:hover,-.fl-page a.fl-button:hover *,
-.fl-responsive-preview-content a.fl-button:hover *,
+.fl-page a.fl-button:hover *,
+.fl-responsive-preview-content a.fl-button:hover *,
+.c-responsive-preview-content a.fl-button:hover *,🤖 Prompt for AI Agents
In themes/beaver/assets/css/theme-main.css around lines 3096 to 3111, the
selector groups for child (*) and :hover blocks are missing
.c-responsive-preview-content which causes inconsistent styling; update the
selector lists at the related blocks (around lines 3171–3205, 3208–3238, and
3240–3267) to include .c-responsive-preview-content wherever similar selectors
appear (e.g., add .c-responsive-preview-content button:visited,
.c-responsive-preview-content input[type="button"],
.c-responsive-preview-content input[type="submit"],
.c-responsive-preview-content a.fl-button and add .c-responsive-preview-content
versions of the :hover selector groups) so the responsive preview content
receives the same child and hover styles as the other contexts.
| (resources.Get "css/footer.css") | ||
| -}} | ||
| {{ partial "assets/css-processor.html" (dict "resources" $clientsResources "bundleName" "single-clients") }} | ||
| {{ partialCached "assets/css-processor.html" (dict "resources" $clientsResources "bundleName" "single-clients") "single-clients" }} |
There was a problem hiding this comment.
Fix cache key to avoid stale per-client CSS
Switching to partialCached with a constant key means the very first client page to render will dictate the cached markup forever. Because this layout builds $dynamicCSS586 via resources.ExecuteAsTemplate ... ., the generated CSS (and its fingerprinted URL) is page-specific. With a static cache key, later client pages will reuse the wrong CSS bundle/link, breaking their styling. Please make the cache key vary per page (e.g., include .RelPermalink or .File.UniqueID) so each client page gets its own cached entry.
- {{ partialCached "assets/css-processor.html" (dict "resources" $clientsResources "bundleName" "single-clients") "single-clients" }}
+ {{ partialCached "assets/css-processor.html" (dict "resources" $clientsResources "bundleName" "single-clients") (printf "single-clients-%s" .RelPermalink) }}📝 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.
| {{ partialCached "assets/css-processor.html" (dict "resources" $clientsResources "bundleName" "single-clients") "single-clients" }} | |
| {{ partialCached "assets/css-processor.html" (dict "resources" $clientsResources "bundleName" "single-clients") (printf "single-clients-%s" .RelPermalink) }} |
🤖 Prompt for AI Agents
In themes/beaver/layouts/clients/single.html around line 15, the partialCached
call uses a static cache key ("single-clients") which causes per-page generated
CSS to be reused across clients; change the cache key to include a page-unique
identifier (for example append .RelPermalink or .File.UniqueID) so the cached
entry is per-client (e.g. build the key with printf or strings.Join like
"single-clients-{{ .File.UniqueID }}" or "single-clients-{{ .RelPermalink }}");
update the last argument of partialCached to that dynamic key so each client
page gets its own cached CSS bundle/link.
Summary by CodeRabbit
New Features
Performance
Security
Style
Documentation
Tests