[604] Updated footer with links and description - #211
Conversation
WalkthroughReplaces the previous simple footer with a new containing .sf-footer__content, a top block with two tagline paragraphs, a links section (LinkedIn, mailto, privacy policy), and a bottom copyright line using {{ data.currentYear or '2025' }} with the ampersand escaped. Adds a new .sf-footer SCSS block with nested elements (.sf-footer__content, __top, __tagline, __links, __links-group, __links-item, __links-item--policy, __bottom) controlling layout, spacing, typography, responsive behavior, and hover states. Adjusts .sf-copy font-size/margins and adds .sf-footer .sf-copy { margin: 0; }. Adds $font-size-footer and $font-size-footer-mobile.Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
🔍 Vulnerabilities of
|
| digest | sha256:37208f4e5187f1131450c3445013a9ab57bad8351b841b2dda74de77a4bbece6 |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 291 MB |
| packages | 984 |
📦 Base Image node:23-alpine
| also known as |
|
| digest | sha256:b9d38d589853406ff0d4364f21969840c3e0397087643aef8eede40edbb6c7cd |
| vulnerabilities |     |
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
website/views/fragments/fragments.html (2)
40-45: Capitalize “LinkedIn” and add an accessible labelBrand name should be capitalized; aria-label clarifies destination and that it opens a new tab.
- <a href="https://www.linkedin.com/company/speed-and-function/" - class="sf-footer__links-item" - target="_blank" - rel="noopener noreferrer"> - linkedin + <a href="https://www.linkedin.com/company/speed-and-function/" + class="sf-footer__links-item" + target="_blank" + rel="noopener noreferrer" + aria-label="Speed and Function on LinkedIn (opens in a new tab)"> + LinkedIn </a>
46-49: Improve accessibility for the email linkConsider an aria-label that explains the action to screen readers.
- <a href="mailto:connect@speedandfunction.com" - class="sf-footer__links-item"> + <a href="mailto:connect@speedandfunction.com" + class="sf-footer__links-item" + aria-label="Email us at connect@speedandfunction.com"> connect@speedandfunction.com </a>website/modules/asset/ui/src/scss/_footer.scss (1)
33-35: Avoid conflicting font declarations between container and items.sf-footer__links sets font-size/weight, but .sf-footer__links-item overrides them. Define typography in one place to reduce specificity battles and future drift.
.sf-footer__links { display: flex; flex-direction: column; align-items: flex-start; gap: 16px; - font-size: $font-size-body-small-mobile; - font-weight: $font-weight-300; + // Typography is defined on link items to avoid conflicts line-height: 120%; } @@ .sf-footer__links-item { - font-size: $font-size-body-small; - font-weight: $font-weight-normal; + font-size: $font-size-body-small; + font-weight: $font-weight-normal; color: $gray-500; text-decoration: none; transition: color 0.2s ease;Alternatively, remove font-size/weight from the item and keep them on the container (but be consistent).
Also applies to: 58-60
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
website/modules/asset/ui/src/scss/_footer.scss(1 hunks)website/views/fragments/fragments.html(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: yuramax
PR: speedandfunction/website#121
File: website/modules/asset/ui/src/scss/_vacancies.scss:99-102
Timestamp: 2025-05-26T15:57:01.766Z
Learning: In website/modules/asset/ui/src/scss/_vacancies.scss, the margin shorthand (e.g., `margin: 0 0 8px`) in `.sf-vacancies_footer h2` is intentionally used to reset all margins and set specific bottom margins, rather than using `margin-bottom` alone. This approach ensures clean baseline styling that doesn't inherit unwanted margins, while still allowing the sibling selector `.vacancy-card + .sf-vacancies_footer h2` to add top margins when a vacancy card precedes the footer.
📚 Learning: 2025-05-26T15:57:01.766Z
Learnt from: yuramax
PR: speedandfunction/website#121
File: website/modules/asset/ui/src/scss/_vacancies.scss:99-102
Timestamp: 2025-05-26T15:57:01.766Z
Learning: In website/modules/asset/ui/src/scss/_vacancies.scss, the margin shorthand (e.g., `margin: 0 0 8px`) in `.sf-vacancies_footer h2` is intentionally used to reset all margins and set specific bottom margins, rather than using `margin-bottom` alone. This approach ensures clean baseline styling that doesn't inherit unwanted margins, while still allowing the sibling selector `.vacancy-card + .sf-vacancies_footer h2` to add top margins when a vacancy card precedes the footer.
Applied to files:
website/modules/asset/ui/src/scss/_footer.scss
⏰ 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). (4)
- GitHub Check: lint
- GitHub Check: e2e-tests
- GitHub Check: unit-tests
- GitHub Check: security-scan
🔇 Additional comments (3)
website/views/fragments/fragments.html (1)
51-53: Confirm Privacy Policy Route and Use Dynamic LinkingFile: website/views/fragments/fragments.html
Lines: 51–53<a href="/privacy-policy" class="sf-footer__links-item"> privacy policy </a>
- Verify that a page is published at “/privacy-policy” and that it correctly incorporates any locale or base-URL prefixes your site uses.
- If this page is managed by your CMS, replace the hardcoded href with your template’s link helper or a configuration value (for example, a
privacyPolicyPage._urlfield orlinkFor('privacyPolicy')helper) so slug, locale and base-URL changes propagate automatically.website/modules/asset/ui/src/scss/_footer.scss (2)
1-4: Footer “sticky to bottom” depends on page layoutmargin-top: auto works only if a flex column layout with min-height: 100vh is applied to the page wrapper. Verify the site shell supports this; otherwise the footer may not stick to the bottom on short pages.
57-69: Ensure accessible focus indication meets contrast requirementsHover/focus only changes color/underline. That’s fine if default focus outline remains. Verify $gray-500 -> $gray-400 provides sufficient contrast at small font sizes, and confirm outline is not disabled elsewhere.
Anton-88
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
website/modules/asset/ui/src/scss/_variables.scss (1)
63-66: DRY the mobile var since it equals desktop (single source of truth).Both variables are set to 11px. Reference the desktop var for the mobile var to simplify future updates.
-// Footer text sizes -$font-size-footer: 11px; -$font-size-footer-mobile: 11px; +// Footer text sizes +$font-size-footer: 11px; +$font-size-footer-mobile: $font-size-footer;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
website/modules/asset/ui/src/scss/_footer.scss(1 hunks)website/modules/asset/ui/src/scss/_variables.scss(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- website/modules/asset/ui/src/scss/_footer.scss
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-26T15:57:01.766Z
Learnt from: yuramax
PR: speedandfunction/website#121
File: website/modules/asset/ui/src/scss/_vacancies.scss:99-102
Timestamp: 2025-05-26T15:57:01.766Z
Learning: In website/modules/asset/ui/src/scss/_vacancies.scss, the margin shorthand (e.g., `margin: 0 0 8px`) in `.sf-vacancies_footer h2` is intentionally used to reset all margins and set specific bottom margins, rather than using `margin-bottom` alone. This approach ensures clean baseline styling that doesn't inherit unwanted margins, while still allowing the sibling selector `.vacancy-card + .sf-vacancies_footer h2` to add top margins when a vacancy card precedes the footer.
Applied to files:
website/modules/asset/ui/src/scss/_variables.scss
⏰ 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). (4)
- GitHub Check: e2e-tests
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: security-scan
…ub.com/speedandfunction/website into 604-update-footer-with-links-and-email # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
website/modules/asset/ui/src/scss/_footer.scss (2)
71-76: BEM modifier for policy spacing looks good (replaces :last-child dependency)This decouples styling from DOM order and is the right move.
83-85: Scoped margin reset for .sf-copy is correctThanks for avoiding a global reset and limiting it to the footer context.
🧹 Nitpick comments (4)
website/modules/asset/ui/src/scss/_footer.scss (4)
20-26: Promote desktop font-size at breakpoint for taglineTagline font-size stays on the mobile token at larger breakpoints. If $font-size-footer is intended for desktop, switch it at breakpoint-medium.
Apply:
.sf-footer__tagline { font-size: $font-size-footer-mobile; font-weight: $font-weight-300; color: $gray-300; margin: 0; line-height: 120%; + @include breakpoint-medium { + font-size: $font-size-footer; + } }
28-43: Use desktop footer font-size at medium breakpoint for linksSame as tagline: font-size remains on the mobile token on desktop. Consider bumping inside the existing breakpoint block.
Apply:
@include breakpoint-medium { flex-direction: row; justify-content: space-between; gap: 32px; text-align: left; + font-size: $font-size-footer; }
57-69: De-duplicate font-size: let items inherit from the containerSince .sf-footer__links already defines the font-size, you can drop the font-size on .sf-footer__links-item and let it inherit. This reduces repetition and keeps desktop scaling centralized (especially if adopting the previous suggestion).
Apply:
.sf-footer__links-item { - font-size: $font-size-footer-mobile; font-weight: $font-weight-normal; color: $gray-500; text-decoration: none; transition: color 0.2s ease;
28-43: Accessibility nit: ensure focus states are clearly visibleYou add underline on :focus, which is good. Consider using :focus-visible specifically to avoid unnecessary styling on mouse focus and ensure keyboard focus is prominent. Also confirm default outlines aren’t suppressed elsewhere globally.
Possible enhancement (if desired):
.sf-footer__links-item { ... - &:hover, - &:focus { + &:hover, + &:focus-visible { color: $gray-400; text-decoration: underline; } }Also applies to: 57-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
website/modules/asset/ui/src/scss/_footer.scss(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-26T15:57:01.766Z
Learnt from: yuramax
PR: speedandfunction/website#121
File: website/modules/asset/ui/src/scss/_vacancies.scss:99-102
Timestamp: 2025-05-26T15:57:01.766Z
Learning: In website/modules/asset/ui/src/scss/_vacancies.scss, the margin shorthand (e.g., `margin: 0 0 8px`) in `.sf-vacancies_footer h2` is intentionally used to reset all margins and set specific bottom margins, rather than using `margin-bottom` alone. This approach ensures clean baseline styling that doesn't inherit unwanted margins, while still allowing the sibling selector `.vacancy-card + .sf-vacancies_footer h2` to add top margins when a vacancy card precedes the footer.
Applied to files:
website/modules/asset/ui/src/scss/_footer.scss
⏰ 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). (4)
- GitHub Check: lint
- GitHub Check: e2e-tests
- GitHub Check: unit-tests
- GitHub Check: security-scan
🔇 Additional comments (2)
website/modules/asset/ui/src/scss/_footer.scss (2)
1-13: Footer container structure and spacing read wellThe base footer block, max-width container, and responsive paddings are coherent and predictable. Margin-top: auto is appropriate for sticky-to-bottom layouts in flex containers.
88-94: .sf-copy is local to the footer — global-regression concern not applicableSearch shows .sf-copy only appears in:
- website/modules/asset/ui/src/scss/_footer.scss (nested .sf-footer .sf-copy and a global .sf-copy block). No other matches found.
You can ignore the original warning about global impact because the class is defined only in the footer file. If you still want to future-proof and scope the font-size to the footer, apply the suggested change (optional):
.sf-footer { .sf-copy { margin: 0; + font-size: $font-size-footer-mobile; + @include breakpoint-medium { + font-size: $font-size-footer; + } } } @@ .sf-copy { text-align: center; font-weight: $font-weight-medium; - font-size: $font-size-body-medium-mobile; + font-size: $font-size-body-small; line-height: 140%; color: $gray-300; }Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
website/modules/asset/ui/src/scss/_footer.scss (5)
18-24: Remove redundant display declaration in .sf-footer__topdisplay: block is declared both outside and inside the breakpoint; the inner one is redundant.
.sf-footer__top { text-align: center; margin-bottom: 32px; display: block; @include breakpoint-medium { - display: block; width: fit-content; margin-left: auto; margin-right: auto; } }
35-44: Simplify .sf-footer__tagline selectorsThe &:first-child rule is redundant, and the extra display: block in &:not(:first-child) is unnecessary given the base rule. Keep the inline-block tweak only at medium+.
- &:first-child { - display: block; - } - &:not(:first-child) { - display: block; @include breakpoint-medium { display: inline-block; } }
55-60: Avoid combining gap with justify-content: space-betweenAt desktop, gap plus space-between compounds spacing unpredictably (gap reserves space; the remaining space is still distributed). If the intent is to push groups to edges and control inner spacing within groups, remove gap here and keep the 32px gap on .sf-footer__links-group.
@include breakpoint-medium { flex-direction: row; justify-content: space-between; - gap: 32px; text-align: left; }
27-33: Legibility check: 11px footer text may be too small on mobile$font-size-footer-mobile appears to be 11px. This is quite small and may hurt readability. If not mandated by design, consider bumping to at least 12–14px on mobile.
47-53: Consider removing duplicated font-size on containerBoth .sf-footer__links and .sf-footer__links-item set font-size: $font-size-footer-mobile. Pick one place (prefer items) to avoid redundant styling unless the container’s font-size is intentionally used for non-link children.
.sf-footer__links { display: flex; flex-direction: column; align-items: flex-start; - font-size: $font-size-footer-mobile; font-weight: $font-weight-300; line-height: 120%;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
website/modules/asset/ui/src/scss/_footer.scss(1 hunks)website/views/fragments/fragments.html(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- website/views/fragments/fragments.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-26T15:57:01.766Z
Learnt from: yuramax
PR: speedandfunction/website#121
File: website/modules/asset/ui/src/scss/_vacancies.scss:99-102
Timestamp: 2025-05-26T15:57:01.766Z
Learning: In website/modules/asset/ui/src/scss/_vacancies.scss, the margin shorthand (e.g., `margin: 0 0 8px`) in `.sf-vacancies_footer h2` is intentionally used to reset all margins and set specific bottom margins, rather than using `margin-bottom` alone. This approach ensures clean baseline styling that doesn't inherit unwanted margins, while still allowing the sibling selector `.vacancy-card + .sf-vacancies_footer h2` to add top margins when a vacancy card precedes the footer.
Applied to files:
website/modules/asset/ui/src/scss/_footer.scss
⏰ 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). (4)
- GitHub Check: e2e-tests
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: security-scan
🔇 Additional comments (2)
website/modules/asset/ui/src/scss/_footer.scss (2)
89-94: Good BEM modifier for policy linkUsing .sf-footer__links-item--policy decouples spacing from DOM order and is more maintainable than :last-child. LGTM.
101-103: Scoped .sf-copy margin reset is correctLimiting margin: 0 to .sf-footer context prevents global regressions. Nice fix.
|
3 participants
Updated footer with links and description