Hero - navbar UI#266
Open
RaphaGil wants to merge 8 commits into
Open
Conversation
✅ Deploy Preview for openms ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughThis PR adds a new modern hero UI (CSS, responsive breakpoints, reduced-motion/contrast rules, animated waveform), site params/config to drive split hero titles and stats, a hero-stats partial and counter JS, new/updated navbar and footer partials, client-side navbar behavior, global typography/design tokens, and multiple CSS updates to use the new variables. ChangesModern Hero Section Feature
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 5
🤖 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/partials/hero.html`:
- Around line 22-24: The primary CTA anchor is currently rendered when only
$buttonText exists, which can produce an invalid link; update the conditional
around the anchor (the block that renders the <a class="hero-modern__btn
hero-modern__btn--primary"...> element) to require both $buttonText and
$buttonLink (e.g., change the if from checking only $buttonText to checking both
$buttonText and $buttonLink) so the button only renders when it has both text
and a valid href.
In `@layouts/partials/navbar-clone.html`:
- Line 1: The template sets $title := .Site.Params.title and uses it as the logo
alt fallback which can be empty; update the fallback to use .Site.Title instead
of .Site.Params.title by changing the variable or the alt usage (references:
$title, .Site.Params.title, .Site.Title, altText) so that when altText is unset
the logo alt uses .Site.Title; apply the same change to the other occurrence
noted (line 9).
- Around line 26-31: The dropdown parent is rendered without an href so it drops
.url and isn't keyboard-navigable; update the anchor in navbar-clone.html inside
the {{- if .sublinks }} block to include href="{{ .url }}" (fall back to "#" if
.url is empty) and add appropriate accessibility attributes (e.g., role="button"
aria-haspopup="true" aria-expanded="false" and tabindex="0") so the element both
links to the configured parent page and supports keyboard/assistive navigation
while keeping the existing .title and {{ if .is_external }}{{ partial "svg-icon"
"launch" }}{{ end }} content.
In `@layouts/partials/navbar.html`:
- Line 1: The fallback for the logo alt text uses $title which is set from
.Site.Params.title and may be empty; change uses of $title (and any alt fallback
logic referencing .Site.Params.title) to use .Site.Title instead so the logo alt
text falls back to the site-wide title; update both the initial declaration
(where $title is assigned) and the other occurrence around the logo markup (the
second instance mentioned) to reference .Site.Title as the fallback for altText.
- Around line 26-31: The dropdown parent anchor rendered inside the {{- if
.sublinks }} block currently drops the configured .url and becomes
non-navigable; update the <a class="navbar-link"> for the parent (inside the
.sublinks branch) to include href="{{ .url }}" so it preserves navigation, keep
the external icon logic using .is_external and partial "svg-icon", and when
.is_external is true add target="_blank" rel="noopener" (and appropriate aria
attributes like aria-haspopup/aria-expanded or tabindex as needed) to restore
keyboard accessibility and correct external-link behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d96a367-a8bd-4865-ac68-091336e7d4e4
📒 Files selected for processing (6)
assets/css/hero.cssassets/css/styles_override.cssconfig.yamllayouts/partials/hero.htmllayouts/partials/navbar-clone.htmllayouts/partials/navbar.html
Comment on lines
+22
to
+24
| {{- if $buttonText }} | ||
| <a class="hero-modern__btn hero-modern__btn--primary" href="{{ $buttonLink }}">{{ $buttonText }}</a> | ||
| {{- end }} |
Contributor
There was a problem hiding this comment.
Guard primary CTA rendering on both text and URL.
Right now the primary button is rendered when buttontext exists, even if buttonlink is missing, which can produce an empty/invalid navigation target.
Suggested fix
- {{- if $buttonText }}
+ {{- if and $buttonText $buttonLink }}
<a class="hero-modern__btn hero-modern__btn--primary" href="{{ $buttonLink }}">{{ $buttonText }}</a>
{{- 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
| {{- if $buttonText }} | |
| <a class="hero-modern__btn hero-modern__btn--primary" href="{{ $buttonLink }}">{{ $buttonText }}</a> | |
| {{- end }} | |
| {{- if and $buttonText $buttonLink }} | |
| <a class="hero-modern__btn hero-modern__btn--primary" href="{{ $buttonLink }}">{{ $buttonText }}</a> | |
| {{- 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/partials/hero.html` around lines 22 - 24, The primary CTA anchor is
currently rendered when only $buttonText exists, which can produce an invalid
link; update the conditional around the anchor (the block that renders the <a
class="hero-modern__btn hero-modern__btn--primary"...> element) to require both
$buttonText and $buttonLink (e.g., change the if from checking only $buttonText
to checking both $buttonText and $buttonLink) so the button only renders when it
has both text and a valid href.
| @@ -0,0 +1,63 @@ | |||
| {{- $title := .Site.Params.title }} | |||
Contributor
There was a problem hiding this comment.
Use .Site.Title for the logo alt fallback.
The fallback title source should be .Site.Title; .Site.Params.title can be empty and produce a broken fallback alt string when altText is not set.
Suggested fix
-{{- $title := .Site.Params.title }}
+{{- $title := .Site.Title }}Also applies to: 9-9
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🤖 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/partials/navbar-clone.html` at line 1, The template sets $title :=
.Site.Params.title and uses it as the logo alt fallback which can be empty;
update the fallback to use .Site.Title instead of .Site.Params.title by changing
the variable or the alt usage (references: $title, .Site.Params.title,
.Site.Title, altText) so that when altText is unset the logo alt uses
.Site.Title; apply the same change to the other occurrence noted (line 9).
| @@ -0,0 +1,63 @@ | |||
| {{- $title := .Site.Params.title }} | |||
Contributor
There was a problem hiding this comment.
Use .Site.Title for the logo alt fallback.
$title is read from .Site.Params.title, which is typically unset here; fallback alt text can degrade to an incorrect string when altText is absent.
Suggested fix
-{{- $title := .Site.Params.title }}
+{{- $title := .Site.Title }}Also applies to: 9-9
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🤖 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/partials/navbar.html` at line 1, The fallback for the logo alt text
uses $title which is set from .Site.Params.title and may be empty; change uses
of $title (and any alt fallback logic referencing .Site.Params.title) to use
.Site.Title instead so the logo alt text falls back to the site-wide title;
update both the initial declaration (where $title is assigned) and the other
occurrence around the logo markup (the second instance mentioned) to reference
.Site.Title as the fallback for altText.
Add sticky site header, news banner styling, brand typography, and hero stats animation while restructuring the navbar for tablet and mobile. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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/hero.css`:
- Line 16: The .hero-modern rule uses overflow: hidden which clips the mobile
stats block that is positioned with bottom: 0 and transform: translateY(100%);
change the overflow so the stats aren't cut off — e.g., remove overflow: hidden
or set overflow-y: visible (or use overflow-x: hidden; overflow-y: visible) on
.hero-modern, and apply the same fix to the related rules around the
mobile-stats styles referenced in the diff (lines ~520-526) so the
absolutely-positioned/styled stats can render off the bottom without being
clipped.
In `@assets/css/mailchimp.css`:
- Around line 84-93: The rule for .sign-up-container .signup-button removes the
default focus outline, which breaks keyboard accessibility on mobile; restore a
visible keyboard focus style by adding a matching .sign-up-container
.signup-button:focus-visible rule (or :focus for broader support) that applies a
clear, contrasty focus indicator (e.g. outline or box-shadow and/or border color
change) and keep the existing border: none/outline: none only for pointer
interactions if needed; update the stylesheet to define .sign-up-container
.signup-button:focus-visible with a visible outline/box-shadow to ensure
keyboard users can see focus.
In `@assets/css/navbar-responsive.css`:
- Line 381: Locate the CSS declaration "fill: currentColor;" (occurs in the
diff) and change the keyword to lowercase so it reads "fill: currentcolor;";
update every occurrence (e.g., in navbar-responsive and styles_override where
that declaration appears) to satisfy stylelint value-keyword-case.
In `@assets/css/news-banner.css`:
- Around line 61-67: The .news-banner__cta CSS rule uses an unreadably small
font-size (0.5rem / 0.4375rem); update the font-size to a more accessible value
(e.g., 0.75rem–0.875rem) in the .news-banner__cta selector and make the same
change for the repeated CTA rule later (the other .news-banner__cta occurrences
mentioned) so the CTA text is consistently larger and readable across screens;
keep font-weight/color/whitespace/transition intact.
In `@assets/js/hero-stats.js`:
- Around line 17-20: The code reads data-count attributes directly and can
produce NaN or invalid decimals; update the parsing around the variables target,
suffix and decimals (where parseFloat and parseInt are used and where
formatValue/animation are called) to validate and provide safe fallbacks: use
Number.isFinite(Number(...)) or isFinite to check parseFloat result and default
target to 0 if invalid, clamp decimals from parseInt to a safe integer range
(e.g., 0–10) and default to 0 if invalid, and keep suffix as the empty string if
missing; apply the same guards where those attributes are parsed later (the
lines referencing target/decimals for animation) so formatValue and any
animation never receive NaN or invalid decimals.
In `@config.yaml`:
- Around line 53-57: The newsBanner configuration is advertising a past event;
update the newsBanner block by either setting enabled: false to hide the banner
or updating text (and link if needed) to reflect a current/future event and
date; specifically modify the newsBanner.enabled and newsBanner.text fields (and
newsBanner.link if the destination changes) so the site no longer displays the
April 30th 2026 workshop as an active/newsworthy item.
In `@layouts/partials/hero-stats.html`:
- Around line 16-18: The template currently injects a <span
class="hero-modern__stat-sep"> directly under the <dl> via the conditional that
checks lt (add $index 1) $statCount; remove that span output (delete the
conditional block that renders hero-modern__stat-sep) and instead implement the
separator in CSS using a pseudo-element on the appropriate child element (e.g.,
target the <dd> or <div> that represents each stat with a selector like
.hero-modern__stat-item:not(:last-child)::after) so separators are rendered
purely with CSS and no non-dt/dd/div children appear under the <dl>.
In `@layouts/partials/navbar-social.html`:
- Around line 5-10: The aria-label currently uses "{{ .icon | humanize }}" which
can be empty when .icon is missing; update the aria-label expression on the
anchor so it falls back to a non-empty value (e.g., .title, .name, .link, or a
literal like "Social link") when .icon is falsy, and keep the existing visual
fallback (the span with class "icon" and the partial "svg-icon") as-is so the
accessible name is always present.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25db8afd-50b6-4843-8f91-c15462e6dd41
⛔ Files ignored due to path filters (1)
assets/icons/linkedin.svgis excluded by!**/*.svg
📒 Files selected for processing (34)
assets/css/brand-palette.cssassets/css/casestudies.cssassets/css/content_override.cssassets/css/hero.cssassets/css/keyfeatures_override.cssassets/css/mailchimp.cssassets/css/members.cssassets/css/navbar-responsive.cssassets/css/news-banner.cssassets/css/quick-facts.cssassets/css/shell.cssassets/css/site-header.cssassets/css/styles_override.cssassets/css/tabs.cssassets/css/terminal.cssassets/css/typography.cssassets/css/vars_override.cssassets/css/webapps.cssassets/js/hero-stats.jsassets/js/navbar.jsconfig.yamlcontent/en/about.mdlayouts/_default/baseof.htmllayouts/index.htmllayouts/partials/css.htmllayouts/partials/footer.htmllayouts/partials/hero-stats.htmllayouts/partials/hero.htmllayouts/partials/navbar-clone.htmllayouts/partials/navbar-cta.htmllayouts/partials/navbar-social.htmllayouts/partials/navbar-sublinks.htmllayouts/partials/navbar.htmllayouts/partials/news-banner.html
💤 Files with no reviewable changes (1)
- assets/css/vars_override.css
✅ Files skipped from review due to trivial changes (3)
- assets/css/members.css
- content/en/about.md
- assets/css/keyfeatures_override.css
|
|
||
| .hero-modern { | ||
| position: relative; | ||
| overflow: hidden; |
Contributor
There was a problem hiding this comment.
Mobile stats can be clipped off-screen by parent overflow.
overflow: hidden on .hero-modern conflicts with mobile stats positioned at bottom: 0 plus translateY(100%), so the stats block can be cut off.
Suggested fix
`@media` (max-width: 768px) {
.hero-modern {
padding: clamp(0.75rem, 1.5vh, 1.5rem) 1.25rem;
min-height: min(28rem, calc(100dvh - var(--openms-hero-chrome)));
+ overflow: visible;
}Also applies to: 520-526
🤖 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/hero.css` at line 16, The .hero-modern rule uses overflow: hidden
which clips the mobile stats block that is positioned with bottom: 0 and
transform: translateY(100%); change the overflow so the stats aren't cut off —
e.g., remove overflow: hidden or set overflow-y: visible (or use overflow-x:
hidden; overflow-y: visible) on .hero-modern, and apply the same fix to the
related rules around the mobile-stats styles referenced in the diff (lines
~520-526) so the absolutely-positioned/styled stats can render off the bottom
without being clipped.
Comment on lines
+84
to
93
| .sign-up-container .signup-button { | ||
| display: block; | ||
| height: 35px; | ||
| border-radius: 5px; | ||
| margin-left: 5px; | ||
| width: 60px; | ||
| color: black; | ||
| border:none; | ||
| outline:none; | ||
| border: none; | ||
| outline: none; | ||
| } |
Contributor
There was a problem hiding this comment.
Restore a visible keyboard focus style for the mobile signup button.
Line [92] removes the focus outline without adding an equivalent :focus-visible treatment in this rule block.
Proposed fix
`@media` only screen and (max-width: 850px) {
.sign-up-container .signup-button {
display: block;
height: 35px;
border-radius: 5px;
margin-left: 5px;
width: 60px;
color: black;
border: none;
outline: none;
}
+
+ .sign-up-container .signup-button:focus-visible {
+ outline: 2px solid var(--openms-blue);
+ outline-offset: 2px;
+ }
}📝 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
| .sign-up-container .signup-button { | |
| display: block; | |
| height: 35px; | |
| border-radius: 5px; | |
| margin-left: 5px; | |
| width: 60px; | |
| color: black; | |
| border:none; | |
| outline:none; | |
| border: none; | |
| outline: none; | |
| } | |
| .sign-up-container .signup-button { | |
| display: block; | |
| height: 35px; | |
| border-radius: 5px; | |
| margin-left: 5px; | |
| width: 60px; | |
| color: black; | |
| border: none; | |
| outline: none; | |
| } | |
| .sign-up-container .signup-button:focus-visible { | |
| outline: 2px solid var(--openms-blue); | |
| outline-offset: 2px; | |
| } |
🤖 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/mailchimp.css` around lines 84 - 93, The rule for
.sign-up-container .signup-button removes the default focus outline, which
breaks keyboard accessibility on mobile; restore a visible keyboard focus style
by adding a matching .sign-up-container .signup-button:focus-visible rule (or
:focus for broader support) that applies a clear, contrasty focus indicator
(e.g. outline or box-shadow and/or border color change) and keep the existing
border: none/outline: none only for pointer interactions if needed; update the
stylesheet to define .sign-up-container .signup-button:focus-visible with a
visible outline/box-shadow to ensure keyboard users can see focus.
| #navbar-clone.navbar.is-fresh .navbar-menu .navbar-mobile-social svg.icon { | ||
| width: 1.5rem; | ||
| height: 1.5rem; | ||
| fill: currentColor; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify remaining value-keyword-case violations for currentColor
rg -n '\bcurrentColor\b' --type cssRepository: OpenMS/OpenMS-website
Length of output: 183
Fix Stylelint value-keyword-case by lowercasing currentColor
fill: currentColor; appears in:
assets/css/navbar-responsive.css:381assets/css/styles_override.css:468
- fill: currentColor;
+ fill: currentcolor;Proposed fix
- fill: currentColor;
+ fill: currentcolor;📝 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
| fill: currentColor; | |
| fill: currentcolor; |
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 381-381: Expected "currentColor" to be "currentcolor" (value-keyword-case)
(value-keyword-case)
🤖 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/navbar-responsive.css` at line 381, Locate the CSS declaration
"fill: currentColor;" (occurs in the diff) and change the keyword to lowercase
so it reads "fill: currentcolor;"; update every occurrence (e.g., in
navbar-responsive and styles_override where that declaration appears) to satisfy
stylelint value-keyword-case.
Comment on lines
+61
to
+67
| .news-banner__cta { | ||
| margin-left: 0.3rem; | ||
| font-size: 0.5rem; | ||
| font-weight: 600; | ||
| color: var(--openms-yellow); | ||
| white-space: nowrap; | ||
| transition: color 0.2s ease; |
Contributor
There was a problem hiding this comment.
Increase CTA font size for readability.
0.5rem / 0.4375rem makes the CTA text extremely hard to read on many screens; this is an accessibility risk for a key interactive element.
Suggested fix
.news-banner__cta {
margin-left: 0.3rem;
- font-size: 0.5rem;
+ font-size: 0.75rem;
font-weight: 600;
color: var(--openms-yellow);
white-space: nowrap;
transition: color 0.2s ease;
}
@@
.news-banner__cta {
- font-size: 0.4375rem;
+ font-size: 0.6875rem;
}Also applies to: 96-98
🤖 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/news-banner.css` around lines 61 - 67, The .news-banner__cta CSS
rule uses an unreadably small font-size (0.5rem / 0.4375rem); update the
font-size to a more accessible value (e.g., 0.75rem–0.875rem) in the
.news-banner__cta selector and make the same change for the repeated CTA rule
later (the other .news-banner__cta occurrences mentioned) so the CTA text is
consistently larger and readable across screens; keep
font-weight/color/whitespace/transition intact.
Comment on lines
+17
to
+20
| var target = parseFloat(counter.getAttribute("data-count-value")); | ||
| var suffix = counter.getAttribute("data-count-suffix") || ""; | ||
| var decimals = parseInt(counter.getAttribute("data-count-decimals") || "0", 10); | ||
| counter.textContent = formatValue(target, decimals) + suffix; |
Contributor
There was a problem hiding this comment.
Guard invalid counter attributes before rendering.
Line 17 and Line 27 parse values without validation. Bad CMS/config data can render NaN (or fail formatting for invalid decimals). Add safe fallbacks before formatting/animating.
Suggested patch
function setFinal(counter) {
- var target = parseFloat(counter.getAttribute("data-count-value"));
+ var target = parseFloat(counter.getAttribute("data-count-value"));
+ if (!Number.isFinite(target)) target = 0;
var suffix = counter.getAttribute("data-count-suffix") || "";
- var decimals = parseInt(counter.getAttribute("data-count-decimals") || "0", 10);
+ var decimals = parseInt(counter.getAttribute("data-count-decimals") || "0", 10);
+ if (!Number.isInteger(decimals) || decimals < 0) decimals = 0;
counter.textContent = formatValue(target, decimals) + suffix;
}
@@
- var target = parseFloat(counter.getAttribute("data-count-value"));
+ var target = parseFloat(counter.getAttribute("data-count-value"));
+ if (!Number.isFinite(target)) target = 0;
var suffix = counter.getAttribute("data-count-suffix") || "";
- var decimals = parseInt(counter.getAttribute("data-count-decimals") || "0", 10);
+ var decimals = parseInt(counter.getAttribute("data-count-decimals") || "0", 10);
+ if (!Number.isInteger(decimals) || decimals < 0) decimals = 0;Also applies to: 27-30
🤖 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/js/hero-stats.js` around lines 17 - 20, The code reads data-count
attributes directly and can produce NaN or invalid decimals; update the parsing
around the variables target, suffix and decimals (where parseFloat and parseInt
are used and where formatValue/animation are called) to validate and provide
safe fallbacks: use Number.isFinite(Number(...)) or isFinite to check parseFloat
result and default target to 0 if invalid, clamp decimals from parseInt to a
safe integer range (e.g., 0–10) and default to 0 if invalid, and keep suffix as
the empty string if missing; apply the same guards where those attributes are
parsed later (the lines referencing target/decimals for animation) so
formatValue and any animation never receive NaN or invalid decimals.
Comment on lines
+53
to
+57
| newsBanner: | ||
| enabled: true | ||
| label: News | ||
| text: Workshop at University of Helsinki on April 30th 2026. | ||
| link: /news/ |
Contributor
There was a problem hiding this comment.
News banner announces a past date.
With enabled: true, the banner currently advertises an event on April 30, 2026, which is already in the past as of May 26, 2026. Please update the text/date or disable the banner to avoid stale homepage messaging.
🤖 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 `@config.yaml` around lines 53 - 57, The newsBanner configuration is
advertising a past event; update the newsBanner block by either setting enabled:
false to hide the banner or updating text (and link if needed) to reflect a
current/future event and date; specifically modify the newsBanner.enabled and
newsBanner.text fields (and newsBanner.link if the destination changes) so the
site no longer displays the April 30th 2026 workshop as an active/newsworthy
item.
Comment on lines
+16
to
+18
| {{- if lt (add $index 1) $statCount }} | ||
| <span class="hero-modern__stat-sep" aria-hidden="true">|</span> | ||
| {{- end }} |
Contributor
There was a problem hiding this comment.
Avoid non-dt/dd/div children directly under <dl>.
Line [17] inserts a <span> directly under <dl>, which is invalid for definition list structure. Render separators with CSS pseudo-elements instead.
Proposed template-side adjustment
- {{- if lt (add $index 1) $statCount }}
- <span class="hero-modern__stat-sep" aria-hidden="true">|</span>
- {{- 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/partials/hero-stats.html` around lines 16 - 18, The template
currently injects a <span class="hero-modern__stat-sep"> directly under the <dl>
via the conditional that checks lt (add $index 1) $statCount; remove that span
output (delete the conditional block that renders hero-modern__stat-sep) and
instead implement the separator in CSS using a pseudo-element on the appropriate
child element (e.g., target the <dd> or <div> that represents each stat with a
selector like .hero-modern__stat-item:not(:last-child)::after) so separators are
rendered purely with CSS and no non-dt/dd/div children appear under the <dl>.
Comment on lines
+5
to
+10
| <a href="{{ .link }}" aria-label="{{ .icon | humanize }}" target="_blank" rel="noopener noreferrer"> | ||
| {{- if .icon -}} | ||
| {{- partial "svg-icon" .icon -}} | ||
| {{- else -}} | ||
| <span class="icon">ⓘ</span> | ||
| {{- end }} |
Contributor
There was a problem hiding this comment.
Ensure each social link always has a non-empty accessible name.
When .icon is missing (Lines [8]-[9]), the current aria-label expression on Line [5] can be empty.
Proposed fix
{{- range $socialMedia }}
- <a href="{{ .link }}" aria-label="{{ .icon | humanize }}" target="_blank" rel="noopener noreferrer">
+ {{- $label := .icon | default "social media link" | humanize }}
+ <a href="{{ .link }}" aria-label="{{ $label }}" target="_blank" rel="noopener noreferrer">
{{- if .icon -}}
{{- partial "svg-icon" .icon -}}
{{- else -}}📝 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
| <a href="{{ .link }}" aria-label="{{ .icon | humanize }}" target="_blank" rel="noopener noreferrer"> | |
| {{- if .icon -}} | |
| {{- partial "svg-icon" .icon -}} | |
| {{- else -}} | |
| <span class="icon">ⓘ</span> | |
| {{- end }} | |
| {{- $label := .icon | default "social media link" | humanize }} | |
| <a href="{{ .link }}" aria-label="{{ $label }}" target="_blank" rel="noopener noreferrer"> | |
| {{- if .icon -}} | |
| {{- partial "svg-icon" .icon -}} | |
| {{- else -}} | |
| <span class="icon">ⓘ</span> | |
| {{- 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/partials/navbar-social.html` around lines 5 - 10, The aria-label
currently uses "{{ .icon | humanize }}" which can be empty when .icon is
missing; update the aria-label expression on the anchor so it falls back to a
non-empty value (e.g., .title, .name, .link, or a literal like "Social link")
when .icon is falsy, and keep the existing visual fallback (the span with class
"icon" and the partial "svg-icon") as-is so the accessible name is always
present.
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
1 participant
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.
Updated the hero section with the new layout/UI as discussed and moved the logo to the navbar.
Summary by CodeRabbit
New Features
Style
Chores