[654] feat(testimonials): Show only domain in testimonial links - #192
Conversation
📝 WalkthroughWalkthroughThe testimonial carousel widget's template was updated to display only the domain name of a testimonial's URL as the anchor text, removing protocol, path, query, fragment, and "www." prefix. The anchor tag now always renders if Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WidgetTemplate
User ->> WidgetTemplate: Render testimonial card
WidgetTemplate ->> WidgetTemplate: Check if card.url exists
alt card.url exists
WidgetTemplate ->> WidgetTemplate: Extract domain from card.url (remove protocol, path, query, fragment, "www.")
WidgetTemplate ->> User: Display anchor with domain as text
else card.url does not exist
WidgetTemplate ->> User: Do not render anchor
end
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
website/modules/testimonials-carousel-widget/views/widget.html (2)
41-46: Domain-extraction logic is brittleParsing with chained
split()works only for URLs that always include a scheme (://).
–//example.com(protocol-relative) or plainexample.comwill fail and fall back to the full URL.
Consider a tiny helper filter (e.g.url_domain) that internally usesurllib.parse.urlsplitso edge cases like ports, credentials, or missing scheme are handled robustly.
44-45: Addrel="noopener noreferrer"for external linksAll links that open in a new tab should set
rel="noopener noreferrer"to avoid thewindow.openersecurity issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/modules/testimonials-carousel-widget/views/widget.html(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: yuramax
PR: speedandfunction/website#120
File: website/modules/asset/ui/src/index.js:43-67
Timestamp: 2025-05-26T06:19:32.209Z
Learning: When reviewing PRs focused on styling, defer functional JavaScript improvements to separate tasks to maintain clear scope separation.
Learnt from: yuramax
PR: speedandfunction/website#139
File: website/jest.config.cjs:4-4
Timestamp: 2025-06-03T16:27:46.172Z
Learning: When reviewing PRs, focus suggestions on the main objectives stated in the PR description. Skip suggestions for incidental changes that are unrelated to the PR's primary purpose, especially when the user explicitly requests to stay focused on the main feature/improvement.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: security-scan
- GitHub Check: e2e-tests
🔍 Vulnerabilities of
|
| digest | sha256:f7d6731243ae85491c7a8e95c138cc72cf4b34d50b2678c23455a20763eb658c |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 290 MB |
| packages | 977 |
📦 Base Image node:23-alpine
| also known as |
|
| digest | sha256:b9d38d589853406ff0d4364f21969840c3e0397087643aef8eede40edbb6c7cd |
| vulnerabilities |    |
Description
| ||||||||||||
Description
| ||||||||||||
Description
|
…s, show domain only, add rel noopener noreferrer
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
website/modules/testimonials-carousel-widget/views/widget.html (2)
41-43: Make URL parsing more robust and easier to readThe chain of string splits relies on the URL containing
://and at least one/after the domain.
Edge-cases such ashttp://localhost:3000, query-only URLs,mailto:links, or already-trimmed domains will break or yield surprising results.Consider centralising the logic in a small filter/helper instead of inline string gymnastics:
-{% set domain = card.url.split('://')|length > 1 and - card.url.split('://')[1].split('/')[0] or card.url %} +{# utils.njk #} +{% macro domain(url) %} + {%- set parts = url.split('://') -%} + {%- set host = (parts|length > 1 and parts[1] or parts[0]).split('/')[0] -%} + {{ host }} +{% endmacro %} + +{# usage #} +{% set domain = domain(card.url) %}Benefits:
• Handles scheme-less,mailto:, or localhost URLs safely.
• Reduces template noise & duplication.
• Improves maintainability if parsing rules change.
44-48: Addrel="nofollow"andaria-labelfor SEO & accessibilitySince these links point to external sites beyond editorial control, adding
rel="nofollow"prevents unintended SEO dilution.
An explicitaria-label(e.g., company domain) makes the link comprehensible to screen-reader users when the anchor text is just the bare domain.-<a - href="{{ card.url }}" - target="_blank" - class="sf-person__url" - rel="noopener noreferrer" -> +<a + href="{{ card.url }}" + target="_blank" + class="sf-person__url" + rel="noopener noreferrer nofollow" + aria-label="Open {{ domain }} in new tab" +>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/modules/testimonials-carousel-widget/views/widget.html(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: yuramax
PR: speedandfunction/website#120
File: website/modules/asset/ui/src/index.js:43-67
Timestamp: 2025-05-26T06:19:32.209Z
Learning: When reviewing PRs focused on styling, defer functional JavaScript improvements to separate tasks to maintain clear scope separation.
Learnt from: yuramax
PR: speedandfunction/website#139
File: website/jest.config.cjs:4-4
Timestamp: 2025-06-03T16:27:46.172Z
Learning: When reviewing PRs, focus suggestions on the main objectives stated in the PR description. Skip suggestions for incidental changes that are unrelated to the PR's primary purpose, especially when the user explicitly requests to stay focused on the main feature/improvement.
🪛 HTMLHint (1.5.0)
website/modules/testimonials-carousel-widget/views/widget.html
[error] 42-42: Special characters must be escaped : [ > ].
(spec-char-escape)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
website/modules/testimonials-carousel-widget/views/widget.html (1)
41-52: Previous invalid-markup issue fixed — good catchWrapping the anchor in
{% if card.url %}eliminates empty<a>elements flagged in earlier reviews.
Implementation now conditionally renders and addsrel="noopener noreferrer", aligning with security best practices.
VitalyyP
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
website/modules/testimonials-carousel-widget/views/widget.html (2)
45-47: Refactor domain-extraction for robustness & readabilityThe current one-liner repeats
split('://')twice and relies on the brittle “and … or …” idiom.
Edge-cases it misses:• protocol-less URLs (“example.com/foo”)
• URLs containing credentials or query params
• inline>character triggers HTMLHint (spec-char-escape) because the comparison operator sits unescaped in the markup.A small refactor makes the intent clearer, avoids repeated work, and silences the linter:
-{% set domain = - card.url.split('://')|length > 1 and - card.url.split('://')[1].split('/')[0] or card.url %} +{# Normalise and extract the host part once #} +{% set urlParts = card.url.split('://') %} +{% set remainder = urlParts|length > 1 ? urlParts[1] : urlParts[0] %} +{% set domain = remainder.split('/')[0] %}No behavioural change, but easier to reason about and HTMLHint-clean.
48-55: Consider addingrel="nofollow"to external testimonial links
noopener noreferreris great, but search crawlers may interpret these dynamically-rendered testimonial links as paid endorsements.
Addingnofollow(orugc) keeps SEO signals explicit:- rel="noopener noreferrer" + rel="noopener noreferrer nofollow"Purely advisory—feel free to skip if your SEO policy differs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/modules/testimonials-carousel-widget/views/widget.html(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: yuramax
PR: speedandfunction/website#120
File: website/modules/asset/ui/src/index.js:43-67
Timestamp: 2025-05-26T06:19:32.209Z
Learning: When reviewing PRs focused on styling, defer functional JavaScript improvements to separate tasks to maintain clear scope separation.
Learnt from: yuramax
PR: speedandfunction/website#139
File: website/jest.config.cjs:4-4
Timestamp: 2025-06-03T16:27:46.172Z
Learning: When reviewing PRs, focus suggestions on the main objectives stated in the PR description. Skip suggestions for incidental changes that are unrelated to the PR's primary purpose, especially when the user explicitly requests to stay focused on the main feature/improvement.
🪛 HTMLHint (1.5.0)
website/modules/testimonials-carousel-widget/views/widget.html
[error] 46-46: Special characters must be escaped : [ > ].
(spec-char-escape)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: e2e-tests
- GitHub Check: security-scan
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/modules/testimonials-carousel-widget/views/widget.html (1)
53-60: Add graceful fallback & accessibility text for the anchor.
cleanDomaincould be empty whencard.urlis malformed.- Screen-reader users don’t get any hint what the link points to.
- <a - href="{{ card.url }}" - target="_blank" - class="sf-person__url" - rel="noopener noreferrer" - > - {{ cleanDomain }} - </a> + {% set anchorText = cleanDomain or card.url %} + <a + href="{{ card.url }}" + target="_blank" + class="sf-person__url" + rel="noopener noreferrer" + aria-label="Open {{ anchorText }} in a new tab" + title="{{ anchorText }}" + > + {{ anchorText }} + </a>This prevents blank anchor text and improves a11y without changing visual
appearance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/modules/testimonials-carousel-widget/views/widget.html(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: yuramax
PR: speedandfunction/website#120
File: website/modules/asset/ui/src/index.js:43-67
Timestamp: 2025-05-26T06:19:32.209Z
Learning: When reviewing PRs focused on styling, defer functional JavaScript improvements to separate tasks to maintain clear scope separation.
Learnt from: yuramax
PR: speedandfunction/website#139
File: website/jest.config.cjs:4-4
Timestamp: 2025-06-03T16:27:46.172Z
Learning: When reviewing PRs, focus suggestions on the main objectives stated in the PR description. Skip suggestions for incidental changes that are unrelated to the PR's primary purpose, especially when the user explicitly requests to stay focused on the main feature/improvement.
🪛 HTMLHint (1.5.0)
website/modules/testimonials-carousel-widget/views/widget.html
[error] 47-47: Special characters must be escaped : [ > ].
(spec-char-escape)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e-tests
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: security-scan
🔇 Additional comments (1)
website/modules/testimonials-carousel-widget/views/widget.html (1)
45-52: Anchor-strip “www.” when extracting domain
- File:
website/modules/testimonials-carousel-widget/views/widget.html, around lines 45–52Replace the unanchored global replace:
-{% set cleanDomain = domainWithOrWithoutWww | trim | replace('www.', '') %} +{# Strip only a leading “www.” #} +{% set cleanDomain = domainWithOrWithoutWww + | trim + | replace('^www\\.', '', regex=true) %}Ensure your Nunjucks version supports the
regex=trueargument; otherwise fall back to a simple prefix check:{% set cleanDomain = domainWithOrWithoutWww %} {% if cleanDomain.startswith('www.') %} {% set cleanDomain = cleanDomain.substr(4) %} {% endif %}
|
5 participants
Uh oh!
There was an error while loading. Please reload this page.