feat: case study success story, immersive styled + revamped index listing page #119
Merged
Merged
Conversation
✅ Deploy Preview for masterpoint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a scoped "immersive" case-study layout: new scoped CSS, an immersive page layout, multiple ChangesImmersive Case Study Layout System
Sequence DiagramsequenceDiagram
participant Content as MarketSpark Content
participant Template as layouts/case-studies/immersive.html
participant Shortcodes as csi-* Shortcodes
participant CSS as assets/css/custom.scss
Content->>Template: page frontmatter (layout: immersive) + .Content
Template->>Template: render hero / stat-strip (modern markup)
Template->>Shortcodes: invoke `csi-split`, `csi-section`, `csi-impact`, `csi-testimonial`
Shortcodes->>Shortcodes: parse args/blocks, RenderString inner markdown, add AOS attrs
Shortcodes->>Template: emit HTML with `csi-*` classes
Template->>CSS: page uses `.case-study-immersive` scope
CSS->>CSS: apply palette, variants, split, cards, testimonials, CTA styles
Template->>Template: append CTA (callout), footer, scripts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
content/case-studies/marketspark-article-style.md (2)
2-2: TODO: Pending MarketSpark logo assets.The TODO notes that proper logo variants (dark & light) are needed from MarketSpark. Once obtained, consider using the new
logoparameter in thecs-aboutshortcode (lines 38-45) for consistent branding.Do you want me to open a tracking issue for this TODO, or help generate placeholder content until the assets are received?
🤖 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 `@content/case-studies/marketspark-article-style.md` at line 2, Add a tracking TODO and placeholder flow for the missing MarketSpark logo assets: open a tracking issue that requests both dark and light logo files and links it in this file, then update the cs-about shortcode usage (cs-about) to accept a new logo parameter and set it to a placeholder SVG/PNG fallback until the real assets arrive; once you get the provided dark & light files, replace the placeholders by assigning the actual logo parameter values to cs-about and remove the TODO/tracking reference.
64-64: 💤 Low valueConsider running
trunk fmtto address style issues.Static analysis detected minor hyphenation style issues:
- Line 64: "zero trust" → "zero-trust"
- Line 94: "per service right sizing" → "per-service right-sizing"
As per coding guidelines, run
trunk fmtto auto-fix these issues.📝 Expected fixes
Line 64:
-- [**Tailscale**](https://tailscale.com/) VPN subnet router architecture for secure, zero trust, identity-aware seamless access to private infrastructure. Replaced legacy VPN that required manual key management and cumbersome configuration. +- [**Tailscale**](https://tailscale.com/) VPN subnet router architecture for secure, zero-trust, identity-aware seamless access to private infrastructure. Replaced legacy VPN that required manual key management and cumbersome configuration.Line 94:
-body: The migration towards AWS Fargate's serverless compute engine enables MarketSpark to run their applications without needing to manage underlying servers. Fargate also reduced compute spend on the same workloads with proper per service right sizing (additionally, [Fargate Spot is used where tolerated, delivering up to a 70% AWS discount](https://aws.amazon.com/fargate/pricing/)). +body: The migration towards AWS Fargate's serverless compute engine enables MarketSpark to run their applications without needing to manage underlying servers. Fargate also reduced compute spend on the same workloads with proper per-service right-sizing (additionally, [Fargate Spot is used where tolerated, delivering up to a 70% AWS discount](https://aws.amazon.com/fargate/pricing/)).Also applies to: 94-94
🤖 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 `@content/case-studies/marketspark-article-style.md` at line 64, Run the formatter (trunk fmt) to auto-fix hyphenation style issues, and manually ensure the two phrases are corrected: change "zero trust" to "zero-trust" in the Tailscale VPN description and change "per service right sizing" to "per-service right-sizing" where that phrase appears; re-run trunk fmt and commit the updated markdown.
🤖 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 `@assets/css/custom.scss`:
- Line 5189: Fix the stylelint failures in assets/css/custom.scss by removing
the stray empty '//' comment lines (delete the lines that only contain '//'),
ensure there is a single blank line before any remaining '//' comment lines to
satisfy the blank-line-before-comment rule, and normalize the CSS color keyword
casing by replacing occurrences of 'currentColor' with the lint-expected
'currentcolor' (search for 'currentColor' tokens in the immersive block). Apply
these fixes for all reported occurrences (lines referenced in the review) so the
immersive block passes stylelint.
- Around line 5583-5591: The stylesheet defines .csi-media but never handles the
emitted modifier .csi-media--contain, so contain="true" has no effect; add a
rule targeting the modifier (e.g., .csi-media--contain or .csi-media.--contain
depending on naming) that applies object-fit: contain (and any necessary
width/height settings) to the inner img (the selector that targets the image
inside .csi-media), ensuring the .csi-media element switches from cover behavior
to contain when the shortcode emits csi-media--contain.
- Around line 5441-5443: The .csi-section__num rule always hides the number so
the csi-section's num prop never shows; update the stylesheet to stop globally
hiding it — either remove the ".csi-section__num { display: none; }" rule or
change it to a visible default like ".csi-section__num { display: inline-block;
}", or make it conditional (e.g. ".csi-section:not([data-num]) .csi-section__num
{ display: none; }") so the num renders when csi-section provides a value.
In `@content/case-studies/marketspark-article-style.md`:
- Line 6: The front matter currently sets draft: false which will publish the
unfinished page; change the front-matter key draft to true (i.e., replace draft:
false with draft: true) so the document is excluded from production
builds—update the top-of-file front matter where the draft field is defined.
In `@content/case-studies/marketspark.md`:
- Line 4: The front matter has a contradictory draft flag: change the YAML key
draft from false to true in the marketspark case study so the doc stays hidden
from production (update the front-matter `draft:` value in
content/case-studies/marketspark.md to `true`, leaving the "NOT READY FOR
RELEASE" note intact).
In `@layouts/case-studies/_markup/render-link.html`:
- Around line 7-8: The external-link detection using strings.HasPrefix
.Destination "http" (variable $external) is too narrow and misses uppercase
schemes and protocol-relative URLs; update the check in render-link.html to
robustly detect external links by either parsing .Destination with net/url
(url.Parse and test parsed.Scheme != "") or by normalizing case and matching
prefixes "http://", "https://", and "//" (e.g.,
strings.HasPrefix(strings.ToLower(.Destination), "http://") ||
strings.HasPrefix(strings.ToLower(.Destination), "https://") ||
strings.HasPrefix(.Destination, "//")); then use that result for adding
target="_blank" rel="noopener noreferrer".
In `@layouts/case-studies/immersive.html`:
- Line 31: The alt attribute's inline template alt="{{ .Params.client | default
"" }} logo" breaks the HTML parser due to the nested quotes; update the template
to produce the client text safely (for example use the default function with
explicit args or a template branch/printf) so the attribute string is formed
without embedded quote tokens — replace the current expression that references
.Params.client and default "" inside the img tag with a parser-safe expression
such as using default with its argument order or using printf/{{ with ...
}}...{{ end }} to emit "(client) logo".
In `@layouts/shortcodes/cs-about.html`:
- Around line 18-22: The img alt currently uses "{{ $name }} logo" which
produces " logo" when $name is empty; update the template that renders the logo
(the block using $logo and $name in the cs-about__logo div) to provide a
fallback for $name—e.g., use the template default for $name (or a conditional)
so the alt becomes something like "Site logo" when $name is missing, ensuring
the alt attribute is always descriptive or intentionally empty for decorative
images.
In `@layouts/shortcodes/csi-steps.html`:
- Around line 11-13: The rendering uses the original index $i from iterating
$blocks but skips empty trimmed blocks ($b), causing skipped step numbers; fix
by computing the sequence from the filtered/non-empty blocks instead of $i —
e.g., filter out empty blocks before the range or introduce a render counter
(like $step) that you initialize before the loop and increment only when $b is
non-empty, then use that counter for labels wherever $i was used; update
references to $i in the template to use the new counter or iterate over the
filtered list so numbering is contiguous.
---
Nitpick comments:
In `@content/case-studies/marketspark-article-style.md`:
- Line 2: Add a tracking TODO and placeholder flow for the missing MarketSpark
logo assets: open a tracking issue that requests both dark and light logo files
and links it in this file, then update the cs-about shortcode usage (cs-about)
to accept a new logo parameter and set it to a placeholder SVG/PNG fallback
until the real assets arrive; once you get the provided dark & light files,
replace the placeholders by assigning the actual logo parameter values to
cs-about and remove the TODO/tracking reference.
- Line 64: Run the formatter (trunk fmt) to auto-fix hyphenation style issues,
and manually ensure the two phrases are corrected: change "zero trust" to
"zero-trust" in the Tailscale VPN description and change "per service right
sizing" to "per-service right-sizing" where that phrase appears; re-run trunk
fmt and commit the updated markdown.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 821b32c2-19ac-40dc-aed9-f55af0420e35
⛔ Files ignored due to path filters (5)
static/img/case-studies/marketspark/aws-organization-docs.pngis excluded by!**/*.pngstatic/img/case-studies/marketspark/marketspark-aws.jpgis excluded by!**/*.jpgstatic/img/case-studies/marketspark/telecommunications.jpgis excluded by!**/*.jpgstatic/img/case-studies/opentofu.jpgis excluded by!**/*.jpgstatic/img/case-studies/spacelift.jpgis excluded by!**/*.jpg
📒 Files selected for processing (21)
assets/css/custom.scsscontent/case-studies/marketspark-article-style.mdcontent/case-studies/marketspark.mdcontent/case-studies/template.mddocs/case-studies.mdlayouts/case-studies/_markup/render-link.htmllayouts/case-studies/immersive.htmllayouts/shortcodes/cs-about.htmllayouts/shortcodes/cs-beforeafter.htmllayouts/shortcodes/cs-callout.htmllayouts/shortcodes/cs-figure.htmllayouts/shortcodes/cs-lockup.htmllayouts/shortcodes/cs-pane.htmllayouts/shortcodes/cs-pullquote.htmllayouts/shortcodes/cs-stats.htmllayouts/shortcodes/cs-todo.htmllayouts/shortcodes/csi-impact.htmllayouts/shortcodes/csi-section.htmllayouts/shortcodes/csi-split.htmllayouts/shortcodes/csi-steps.htmllayouts/shortcodes/csi-testimonial.html
💤 Files with no reviewable changes (8)
- layouts/shortcodes/cs-todo.html
- layouts/shortcodes/cs-lockup.html
- content/case-studies/template.md
- layouts/shortcodes/cs-figure.html
- layouts/shortcodes/cs-stats.html
- layouts/shortcodes/cs-callout.html
- layouts/shortcodes/cs-beforeafter.html
- layouts/shortcodes/cs-pane.html
| // hero + at-a-glance stat strip reuse the `.case-study-modern .cs-*` rules above | ||
| // (top is byte-identical to modern). EVERYTHING here is scoped to `.csi-*` — | ||
| // never bare element selectors — so it can't touch the modern hero/stats. | ||
| // |
Contributor
There was a problem hiding this comment.
Resolve the stylelint errors in the immersive block.
The current changes introduce lint failures (empty comment, missing blank lines before // comments, and currentColor casing). This will keep style checks red.
Also applies to: 5210-5210, 5218-5218, 5337-5337, 5348-5348, 5404-5404, 5415-5415, 5444-5444, 5473-5473, 5560-5560, 5599-5599
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 5189-5189: Unexpected empty comment (scss/comment-no-empty)
(scss/comment-no-empty)
🤖 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 `@assets/css/custom.scss` at line 5189, Fix the stylelint failures in
assets/css/custom.scss by removing the stray empty '//' comment lines (delete
the lines that only contain '//'), ensure there is a single blank line before
any remaining '//' comment lines to satisfy the blank-line-before-comment rule,
and normalize the CSS color keyword casing by replacing occurrences of
'currentColor' with the lint-expected 'currentcolor' (search for 'currentColor'
tokens in the immersive block). Apply these fixes for all reported occurrences
(lines referenced in the review) so the immersive block passes stylelint.
Comment on lines
+5441
to
+5443
| .csi-section__num { | ||
| display: none; | ||
| } |
Contributor
There was a problem hiding this comment.
num is currently non-functional because it is always hidden.
csi-section accepts a num parameter, but .csi-section__num { display: none; } prevents it from ever rendering visually.
Suggested fix
- .csi-section__num {
- display: none;
- }
+ .csi-section__num {
+ position: absolute;
+ top: 0;
+ right: 0;
+ font-size: clamp(2.4rem, 7vw, 4.8rem);
+ line-height: 1;
+ font-weight: 900;
+ opacity: 0.1;
+ pointer-events: none;
+ }📝 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.
Suggested change
| .csi-section__num { | |
| display: none; | |
| } | |
| .csi-section__num { | |
| position: absolute; | |
| top: 0; | |
| right: 0; | |
| font-size: clamp(2.4rem, 7vw, 4.8rem); | |
| line-height: 1; | |
| font-weight: 900; | |
| opacity: 0.1; | |
| pointer-events: none; | |
| } |
🤖 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 `@assets/css/custom.scss` around lines 5441 - 5443, The .csi-section__num rule
always hides the number so the csi-section's num prop never shows; update the
stylesheet to stop globally hiding it — either remove the ".csi-section__num {
display: none; }" rule or change it to a visible default like ".csi-section__num
{ display: inline-block; }", or make it conditional (e.g.
".csi-section:not([data-num]) .csi-section__num { display: none; }") so the num
renders when csi-section provides a value.
Comment on lines
+5583
to
+5591
| .csi-media { | ||
| margin: 0; | ||
| img { | ||
| display: block; | ||
| width: 100%; | ||
| height: auto; | ||
| border-radius: 14px; | ||
| } | ||
| &__caption { |
Contributor
There was a problem hiding this comment.
contain="true" has no effect without a .csi-media--contain rule.
The shortcode emits csi-media--contain, but this stylesheet never defines behavior for it, so media always stays in cover mode.
Suggested fix
.csi-media {
margin: 0;
img {
display: block;
width: 100%;
height: auto;
border-radius: 14px;
}
+ &--contain img {
+ object-fit: contain;
+ background: rgba(255, 255, 255, 0.04);
+ padding: 0.6rem;
+ }
&__caption {
margin-top: 0.9rem;
font-size: 0.85rem;📝 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.
Suggested change
| .csi-media { | |
| margin: 0; | |
| img { | |
| display: block; | |
| width: 100%; | |
| height: auto; | |
| border-radius: 14px; | |
| } | |
| &__caption { | |
| .csi-media { | |
| margin: 0; | |
| img { | |
| display: block; | |
| width: 100%; | |
| height: auto; | |
| border-radius: 14px; | |
| } | |
| &--contain img { | |
| object-fit: contain; | |
| background: rgba(255, 255, 255, 0.04); | |
| padding: 0.6rem; | |
| } | |
| &__caption { |
🤖 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 `@assets/css/custom.scss` around lines 5583 - 5591, The stylesheet defines
.csi-media but never handles the emitted modifier .csi-media--contain, so
contain="true" has no effect; add a rule targeting the modifier (e.g.,
.csi-media--contain or .csi-media.--contain depending on naming) that applies
object-fit: contain (and any necessary width/height settings) to the inner img
(the selector that targets the image inside .csi-media), ensuring the .csi-media
element switches from cover behavior to contain when the shortcode emits
csi-media--contain.
| # TODO: NEED TO ASK MARKETSPARK FOR PROPER LOGO (dark & light variants) | ||
| draft: true # NOT READY FOR RELEASE | ||
| # TODO: replace placeholder testimonial with a real MarketSpark quote from the retro | ||
| draft: false # NOT READY FOR RELEASE |
Contributor
There was a problem hiding this comment.
Contradictory draft status.
The comment says NOT READY FOR RELEASE, but draft: false means this page will be published to production. Per coding guidelines, content that should be hidden from production builds must set draft: true.
🛡️ Proposed fix
-draft: false # NOT READY FOR RELEASE
+draft: true # NOT READY FOR RELEASEAs per coding guidelines: "Set draft: true in front matter to hide content from production builds."
📝 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.
Suggested change
| draft: false # NOT READY FOR RELEASE | |
| draft: true # NOT READY FOR RELEASE |
🤖 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 `@content/case-studies/marketspark.md` at line 4, The front matter has a
contradictory draft flag: change the YAML key draft from false to true in the
marketspark case study so the doc stays hidden from production (update the
front-matter `draft:` value in content/case-studies/marketspark.md to `true`,
leaving the "NOT READY FOR RELEASE" note intact).
Comment on lines
+7
to
+8
| {{- $external := strings.HasPrefix .Destination "http" -}} | ||
| <a href="{{ .Destination | safeURL }}"{{ with .Title }} title="{{ . }}"{{ end }}{{ if $external }} target="_blank" rel="noopener noreferrer"{{ end }}>{{ .Text | safeHTML }}</a> |
Contributor
There was a problem hiding this comment.
External-link detection is too narrow.
HasPrefix(..., "http") misses uppercase/protocol-relative forms, so some external links won’t get target/rel.
Suggested fix
-{{- $external := strings.HasPrefix .Destination "http" -}}
+{{- $dest := lower .Destination -}}
+{{- $external := or (strings.HasPrefix $dest "http://") (strings.HasPrefix $dest "https://") (strings.HasPrefix $dest "//") -}}📝 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.
Suggested change
| {{- $external := strings.HasPrefix .Destination "http" -}} | |
| <a href="{{ .Destination | safeURL }}"{{ with .Title }} title="{{ . }}"{{ end }}{{ if $external }} target="_blank" rel="noopener noreferrer"{{ end }}>{{ .Text | safeHTML }}</a> | |
| {{- $dest := lower .Destination -}} | |
| {{- $external := or (strings.HasPrefix $dest "http://") (strings.HasPrefix $dest "https://") (strings.HasPrefix $dest "//") -}} | |
| <a href="{{ .Destination | safeURL }}"{{ with .Title }} title="{{ . }}"{{ end }}{{ if $external }} target="_blank" rel="noopener noreferrer"{{ end }}>{{ .Text | safeHTML }}</a> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 8-8: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 8-8: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 8-8: Tag must be paired, no start tag: [ ]
(tag-pair)
🤖 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 `@layouts/case-studies/_markup/render-link.html` around lines 7 - 8, The
external-link detection using strings.HasPrefix .Destination "http" (variable
$external) is too narrow and misses uppercase schemes and protocol-relative
URLs; update the check in render-link.html to robustly detect external links by
either parsing .Destination with net/url (url.Parse and test parsed.Scheme !=
"") or by normalizing case and matching prefixes "http://", "https://", and "//"
(e.g., strings.HasPrefix(strings.ToLower(.Destination), "http://") ||
strings.HasPrefix(strings.ToLower(.Destination), "https://") ||
strings.HasPrefix(.Destination, "//")); then use that result for adding
target="_blank" rel="noopener noreferrer".
| {{ if .Params.client_logo }} | ||
| <div class="cs-hero__lockup"> | ||
| <div class="cs-hero__lockup-mark cs-hero__lockup-mark--client"> | ||
| <img src="{{ .Params.client_logo }}" alt="{{ .Params.client | default "" }} logo" /> |
Contributor
There was a problem hiding this comment.
Rewrite the alt template expression to avoid HTML parser breakage.
This line is currently tripping the formatter/parser (img tag seen as unterminated). The nested default "" expression inside the attribute should be rewritten to a parser-safe template branch.
Suggested fix
- <img src="{{ .Params.client_logo }}" alt="{{ .Params.client | default "" }} logo" />
+ <img src="{{ .Params.client_logo }}" alt="{{ with .Params.client }}{{ . }} logo{{ else }}Client logo{{ 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.
Suggested change
| <img src="{{ .Params.client_logo }}" alt="{{ .Params.client | default "" }} logo" /> | |
| <img src="{{ .Params.client_logo }}" alt="{{ with .Params.client }}{{ . }} logo{{ else }}Client logo{{ end }}" /> |
🧰 Tools
🪛 GitHub Check: Trunk Check
[failure] 31-31: prettier(SyntaxError)
[new] Opening tag "img" not terminated.
🪛 HTMLHint (1.9.2)
[error] 31-31: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 31-31: Special characters must be escaped : [ > ].
(spec-char-escape)
🤖 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 `@layouts/case-studies/immersive.html` at line 31, The alt attribute's inline
template alt="{{ .Params.client | default "" }} logo" breaks the HTML parser due
to the nested quotes; update the template to produce the client text safely (for
example use the default function with explicit args or a template branch/printf)
so the attribute string is formed without embedded quote tokens — replace the
current expression that references .Params.client and default "" inside the img
tag with a parser-safe expression such as using default with its argument order
or using printf/{{ with ... }}...{{ end }} to emit "(client) logo".
Comment on lines
+18
to
+22
| {{- if $logo -}} | ||
| <div class="cs-about__logo"> | ||
| <img src="{{ $logo }}" alt="{{ $name }} logo" /> | ||
| </div> | ||
| {{- end -}} |
Contributor
There was a problem hiding this comment.
Handle empty name parameter in logo alt text.
If the name parameter is not provided or is empty, the alt text on line 20 will render as just " logo" (space + "logo"), which is not descriptive. Consider providing a fallback for the alt attribute.
♿ Proposed fix for accessible alt text
{{- if $logo -}}
<div class="cs-about__logo">
- <img src="{{ $logo }}" alt="{{ $name }} logo" />
+ <img src="{{ $logo }}" alt="{{ cond (ne $name "") (printf "%s logo" $name) "Client logo" }}" />
</div>
{{- end -}}🤖 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 `@layouts/shortcodes/cs-about.html` around lines 18 - 22, The img alt currently
uses "{{ $name }} logo" which produces " logo" when $name is empty; update the
template that renders the logo (the block using $logo and $name in the
cs-about__logo div) to provide a fallback for $name—e.g., use the template
default for $name (or a conditional) so the alt becomes something like "Site
logo" when $name is missing, ensuring the alt attribute is always descriptive or
intentionally empty for decorative images.
Comment on lines
+11
to
+13
| {{- range $i, $block := $blocks -}} | ||
| {{- $b := trim $block " \t\n" -}} | ||
| {{- if $b -}} |
Contributor
There was a problem hiding this comment.
Step numbers can skip when empty blocks are present.
Index $i comes from all split blocks, but rendering skips empty ones afterward. That can produce non-contiguous labels.
Suggested fix
<div class="csi-steps">
- {{- range $i, $block := $blocks -}}
+ {{- $step := 0 -}}
+ {{- range $block := $blocks -}}
{{- $b := trim $block " \t\n" -}}
{{- if $b -}}
+ {{- $step = add $step 1 -}}
{{- $lines := split $b "\n" -}}
@@
- <span class="csi-step__num">{{ printf "%02d" (add $i 1) }}</span>
+ <span class="csi-step__num">{{ printf "%02d" $step }}</span>Also applies to: 28-28
🤖 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 `@layouts/shortcodes/csi-steps.html` around lines 11 - 13, The rendering uses
the original index $i from iterating $blocks but skips empty trimmed blocks
($b), causing skipped step numbers; fix by computing the sequence from the
filtered/non-empty blocks instead of $i — e.g., filter out empty blocks before
the range or introduce a render counter (like $step) that you initialize before
the loop and increment only when $b is non-empty, then use that counter for
labels wherever $i was used; update references to $i in the template to use the
new counter or iterate over the filtered list so numbering is contiguous.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
content/case-studies/marketspark.md (1)
1-1:⚠️ Potential issue | 🟡 MinorRun Trunk lint for the changed case-study markdown
CI runs
trunkchecks on PRs via.github/workflows/lint.yaml. For this specific file undercontent/case-studies/**/*.md,.trunk/trunk.yamldisablesmarkdownlint/prettierchecks, so any failures would come from other enabled linters—still runtrunk check, and usetrunk fmtif you have Trunk available to apply any auto-fixes.🤖 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 `@content/case-studies/marketspark.md` at line 1, Run trunk check against the changed case study (marketspark.md) to surface any CI lint failures and, if you have Trunk locally, run trunk fmt on the same file to apply auto-fixes; since .trunk/trunk.yaml disables markdownlint/prettier for content/case-studies/**/*.md, expect failures only from other enabled linters and commit any formatting fixes produced by trunk fmt before pushing.Source: Coding guidelines
🤖 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 `@content/case-studies/marketspark.md`:
- Line 128: Update the compound modifier in the body text so “customer facing
Production workloads” becomes “customer-facing Production workloads”; edit the
line containing the body: sentence (the phrase starting "A misconfiguration,
runaway process, or compromised credential...") and hyphenate "customer-facing"
to correct the compound adjective usage.
---
Outside diff comments:
In `@content/case-studies/marketspark.md`:
- Line 1: Run trunk check against the changed case study (marketspark.md) to
surface any CI lint failures and, if you have Trunk locally, run trunk fmt on
the same file to apply auto-fixes; since .trunk/trunk.yaml disables
markdownlint/prettier for content/case-studies/**/*.md, expect failures only
from other enabled linters and commit any formatting fixes produced by trunk fmt
before pushing.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 35d65d43-b1f7-4420-902e-502d037c9636
📒 Files selected for processing (4)
content/case-studies/marketspark-article-style.mdcontent/case-studies/marketspark.mddocs/case-studies.mdlayouts/case-studies/immersive.html
🚧 Files skipped from review as they are similar to previous changes (2)
- content/case-studies/marketspark-article-style.md
- layouts/case-studies/immersive.html
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
assets/css/custom.scss (2)
5529-5530:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd blank line before the comment to satisfy stylelint.
The comment at line 5529 needs a blank line above it per the
scss/double-slash-comment-empty-line-beforerule.🧹 Proposed fix
strong { color: $csi-ink; font-weight: 800; } + // `:not(.csi-list)` — the compact csi-list ul carries its own grid layout; // keep the prose bullet/spacing treatment off itAs per coding guidelines, run
trunk fmtto auto-fix linting issues in SCSS files.🤖 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 `@assets/css/custom.scss` around lines 5529 - 5530, The SCSS linter rule requires a blank line before the single-line comment that starts with "// `:not(.csi-list)` — the compact csi-list ul carries its own grid layout;": add a single empty line immediately above that comment in assets/css/custom.scss (the comment containing ":not(.csi-list)") so it satisfies scss/double-slash-comment-empty-line-before, then run trunk fmt to auto-format and re-run the linter.Sources: Coding guidelines, Linters/SAST tools
5785-5786:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd blank line before the comment to satisfy stylelint.
The comment at line 5785 needs a blank line above it per the
scss/double-slash-comment-empty-line-beforerule.🧹 Proposed fix
} } } + // --------------------------------------------------------------------------- // LIST — compact two-column icon rows; the space-saving sibling of csi-impactAs per coding guidelines, run
trunk fmtto auto-fix linting issues in SCSS files.🤖 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 `@assets/css/custom.scss` around lines 5785 - 5786, Add a blank line immediately above the comment that starts with "// ---------------------------------------------------------------------------" and the "LIST — compact two-column icon rows" line to satisfy the scss/double-slash-comment-empty-line-before rule; locate the comment block (the two consecutive single-line // comments shown in the diff) and insert one empty line above it, then run the project's SCSS formatter (e.g., trunk fmt) to ensure the file (assets/css/custom.scss) conforms to stylelint rules.Sources: Coding guidelines, Linters/SAST tools
content/case-studies/marketspark.md (2)
128-128:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHyphenate "customer-facing" for correctness.
Use "customer-facing Production workloads" to fix the compound modifier.
📝 Proposed fix
-body: A misconfiguration, runaway process, or compromised credential in a lower environment (e.g. Staging/QA) can no longer reach customer facing Production workloads. Each environment runs in its own AWS account with its own IAM scope and isolated VPC networking. +body: A misconfiguration, runaway process, or compromised credential in a lower environment (e.g. Staging/QA) can no longer reach customer-facing Production workloads. Each environment runs in its own AWS account with its own IAM scope and isolated VPC networking.🤖 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 `@content/case-studies/marketspark.md` at line 128, In the frontmatter 'body' text, change the compound modifier "customer facing Production workloads" to use a hyphen: "customer-facing Production workloads" so the phrase is grammatically correct; update the string value that currently reads "A misconfiguration, runaway process, or compromised credential in a lower environment (e.g. Staging/QA) can no longer reach customer facing Production workloads." to the hyphenated form.
4-4:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winContradictory draft status.
The comment says
NOT READY FOR RELEASE, butdraft: falsemeans this page will be published to production.🛡️ Proposed fix
-draft: false # NOT READY FOR RELEASE +draft: true # NOT READY FOR RELEASEAs per coding guidelines: "Set
draft: truein front matter to hide content from production builds."🤖 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 `@content/case-studies/marketspark.md` at line 4, The frontmatter in marketspark.md incorrectly sets draft: false while the comment indicates "NOT READY FOR RELEASE"; update the frontmatter key draft to true (change draft: false to draft: true) so the page is hidden from production builds and aligns with the comment.Source: Coding guidelines
🤖 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 `@layouts/shortcodes/csi-list.html`:
- Around line 30-32: The current continuation condition appends any
non-directive line to $body (the clause using "else if and $line $body"),
causing stray/typo lines to be swallowed; change the logic to only append when
the previous parsed key was "body" (introduce and set a flag like $lastKey or
$inBody when you parse "body:" and check it here: only perform "$body = printf
"%s %s" $body $line" if $lastKey == "body"), or alternatively add explicit
validation of recognized directives and emit a warning for unrecognized keys
instead of appending; update the template branch that references $body and $line
accordingly so continuation is restricted to genuine body continuations.
---
Duplicate comments:
In `@assets/css/custom.scss`:
- Around line 5529-5530: The SCSS linter rule requires a blank line before the
single-line comment that starts with "// `:not(.csi-list)` — the compact
csi-list ul carries its own grid layout;": add a single empty line immediately
above that comment in assets/css/custom.scss (the comment containing
":not(.csi-list)") so it satisfies scss/double-slash-comment-empty-line-before,
then run trunk fmt to auto-format and re-run the linter.
- Around line 5785-5786: Add a blank line immediately above the comment that
starts with "//
---------------------------------------------------------------------------" and
the "LIST — compact two-column icon rows" line to satisfy the
scss/double-slash-comment-empty-line-before rule; locate the comment block (the
two consecutive single-line // comments shown in the diff) and insert one empty
line above it, then run the project's SCSS formatter (e.g., trunk fmt) to ensure
the file (assets/css/custom.scss) conforms to stylelint rules.
In `@content/case-studies/marketspark.md`:
- Line 128: In the frontmatter 'body' text, change the compound modifier
"customer facing Production workloads" to use a hyphen: "customer-facing
Production workloads" so the phrase is grammatically correct; update the string
value that currently reads "A misconfiguration, runaway process, or compromised
credential in a lower environment (e.g. Staging/QA) can no longer reach customer
facing Production workloads." to the hyphenated form.
- Line 4: The frontmatter in marketspark.md incorrectly sets draft: false while
the comment indicates "NOT READY FOR RELEASE"; update the frontmatter key draft
to true (change draft: false to draft: true) so the page is hidden from
production builds and aligns with the comment.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 613588ee-8bff-4d9a-a0d0-4026d4339eb8
📒 Files selected for processing (5)
assets/css/custom.scsscontent/case-studies/marketspark.mddocs/case-studies.mdlayouts/shortcodes/csi-list.htmllayouts/shortcodes/csi-section.html
🚧 Files skipped from review as they are similar to previous changes (2)
- layouts/shortcodes/csi-section.html
- docs/case-studies.md
Comment on lines
+30
to
+32
| {{- else if and $line $body -}} | ||
| {{- $body = printf "%s %s" $body $line -}} | ||
| {{- end -}} |
Contributor
There was a problem hiding this comment.
Line continuation logic may append unintended content.
The template appends any non-directive line to $body once a body exists. If an author adds a blank directive or typo (e.g., icons: instead of icon:), it silently becomes part of the previous item's body instead of failing loudly.
Example:
body: Main text.
icons: fa-typo ← typo, gets appended to body
Consider restricting line continuation to only lines that immediately follow body: or add validation to warn on unrecognized directives. Alternatively, document this behavior in the comment block so authors know trailing lines are always appended.
🤖 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 `@layouts/shortcodes/csi-list.html` around lines 30 - 32, The current
continuation condition appends any non-directive line to $body (the clause using
"else if and $line $body"), causing stray/typo lines to be swallowed; change the
logic to only append when the previous parsed key was "body" (introduce and set
a flag like $lastKey or $inBody when you parse "body:" and check it here: only
perform "$body = printf "%s %s" $body $line" if $lastKey == "body"), or
alternatively add explicit validation of recognized directives and emit a
warning for unrecognized keys instead of appending; update the template branch
that references $body and $line accordingly so continuation is restricted to
genuine body continuations.
oycyc
changed the title
This was referenced Jun 20, 2026
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
2 participants
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.
Primary items in this PR:
✅ Approval on Slack: https://masterpoint.slack.com/archives/C0B9UE13C05/p1781613401380519?thread_ts=1781286627.209939&cid=C0B9UE13C05
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation