[767] fix: Resolve testimonials being cut - #210
Conversation
WalkthroughThe PR updates testimonial SCSS. In website/modules/asset/ui/src/scss/_testimonials.scss, .sf-person__content no longer uses truncate-lines(7); it now uses a fixed max-height (150px) with vertical scrolling (overflow-y: auto), prevents horizontal overflow, enables wrap/break behavior, iOS scrolling optimization, overscroll containment, stable scrollbar gutter, and adds 12px right padding. For single-item testimonials (data-testimonial-count='1'), the previous 10-line truncation inside .sf-person-wrapper is removed. In website/modules/asset/ui/src/scss/_carousel.scss, some selector formatting was adjusted and a breakpoint-based truncate-lines(8) on .sf-person__content was removed. Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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:2c30517610d617458857033d3d7b94e4e224e87b03067edafed64c1684636d23 |
| 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/modules/asset/ui/src/scss/_testimonials.scss(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: yuramax
PR: speedandfunction/website#191
File: website/modules/asset/ui/src/scss/_leadership-team.scss:213-216
Timestamp: 2025-07-02T08:15:36.173Z
Learning: In website/modules/asset/ui/src/scss/_leadership-team.scss, the `.leader-bio` element intentionally uses `overflow-y: hidden` at the `breakpoint-extra-large` level to completely remove scrollbars on desktop devices, while maintaining `overflow-y: auto` at the `breakpoint-large` level for tablets. This design decision relies on content managers controlling bio text length to prevent overflow on desktop, ensuring a clean scrollbar-free experience on larger screens.
Learnt from: IhorMasechko
PR: speedandfunction/website#132
File: website/modules/asset/ui/src/scss/_not-found.scss:52-64
Timestamp: 2025-05-29T07:16:52.843Z
Learning: In website/modules/asset/ui/src/scss/_not-found.scss, the .two-buttons container with flex-direction: row and child .sf-button elements having width: 100% does not cause overflow issues and renders correctly, despite theoretical expectations.
📚 Learning: 2025-07-02T08:15:36.173Z
Learnt from: yuramax
PR: speedandfunction/website#191
File: website/modules/asset/ui/src/scss/_leadership-team.scss:213-216
Timestamp: 2025-07-02T08:15:36.173Z
Learning: In website/modules/asset/ui/src/scss/_leadership-team.scss, the `.leader-bio` element intentionally uses `overflow-y: hidden` at the `breakpoint-extra-large` level to completely remove scrollbars on desktop devices, while maintaining `overflow-y: auto` at the `breakpoint-large` level for tablets. This design decision relies on content managers controlling bio text length to prevent overflow on desktop, ensuring a clean scrollbar-free experience on larger screens.
Applied to files:
website/modules/asset/ui/src/scss/_testimonials.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: unit-tests
- GitHub Check: lint
- GitHub Check: e2e-tests
- GitHub Check: security-scan
VitalyyP
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
website/modules/asset/ui/src/scss/_carousel.scss(2 hunks)website/modules/asset/ui/src/scss/_testimonials.scss(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- website/modules/asset/ui/src/scss/_testimonials.scss
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: yuramax
PR: speedandfunction/website#191
File: website/modules/asset/ui/src/scss/_leadership-team.scss:213-216
Timestamp: 2025-07-02T08:15:36.173Z
Learning: In website/modules/asset/ui/src/scss/_leadership-team.scss, the `.leader-bio` element intentionally uses `overflow-y: hidden` at the `breakpoint-extra-large` level to completely remove scrollbars on desktop devices, while maintaining `overflow-y: auto` at the `breakpoint-large` level for tablets. This design decision relies on content managers controlling bio text length to prevent overflow on desktop, ensuring a clean scrollbar-free experience on larger screens.
⏰ 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
🔇 Additional comments (2)
website/modules/asset/ui/src/scss/_carousel.scss (2)
47-55: Whitespace-only selector change — OKSpacing before the brace is a no-op. No behavioral impact.
56-66: Whitespace-only selector change — OKNo functional differences introduced here.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
website/modules/asset/ui/src/scss/_testimonials.scss (2)
69-71: Medium breakpoint still caps content at 150px due to base rule; allow content to use the 240px wrapper.At medium,
.sf-person-wrapperisheight: 240px, but.sf-person__contentremains constrained by the basemax-height: 150px. This leaves unused space and may still feel “cut off”.If percentages are problematic in your layout, set an explicit max-height at the breakpoint to avoid using
height: 100%.Apply this diff inside the existing breakpoint block:
@include breakpoint-medium { - height: 240px; + height: 240px; + .sf-person__content { + /* Let content utilize the full wrapper height at medium */ + max-height: 240px; /* explicit to avoid percentage sizing issues */ + overflow-y: auto; + overflow-x: hidden; + } }
146-151: Single-item (400px) variant: content is still capped at 150px; mirror the wrapper’s 400px height.In the single-item layout, the wrapper is 400px at medium, but
.sf-person__contentdoesn’t override the basemax-height: 150px. Explicitly set the max-height to match the wrapper so longer testimonials can scroll within the 400px space.Apply this diff:
@include breakpoint-medium { height: 400px; .sf-person__content { + /* Allow scrolling within the 400px wrapper */ + max-height: 400px; /* avoids % heights */ + overflow-y: auto; + overflow-x: hidden; @include font-settings(22px, 150%, 400); } }
🧹 Nitpick comments (2)
website/modules/asset/ui/src/scss/_testimonials.scss (2)
82-85: Nit: scope overscroll behavior to Y-axis; revisit both-edges gutter.
- Prefer
overscroll-behavior-y: contain;to avoid unintentionally affecting horizontal gestures/interactions (e.g., Swiper drag) at this node.scrollbar-gutter: stable both-edges;reserves space on both sides; on narrow mobile viewports and overlay-scrollbar OSes it can reduce available text width more than needed. Considerstable(inline-end only).Apply this diff:
- overscroll-behavior: contain; - scrollbar-gutter: stable both-edges; + overscroll-behavior-y: contain; + scrollbar-gutter: stable;
75-86: Accessibility follow-up: ensure the scrollable region is keyboard-focusable.To make the scrollable content accessible to keyboard users/screen readers, ensure the markup provides a focus target (e.g.,
tabindex="0") and an accessible name (e.g.,role="region"witharia-label). SCSS looks good; this requires a template change.I can draft the minimal markup change and ARIA guidance if helpful.
📜 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/_testimonials.scss(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: yuramax
PR: speedandfunction/website#191
File: website/modules/asset/ui/src/scss/_leadership-team.scss:213-216
Timestamp: 2025-07-02T08:15:36.173Z
Learning: In website/modules/asset/ui/src/scss/_leadership-team.scss, the `.leader-bio` element intentionally uses `overflow-y: hidden` at the `breakpoint-extra-large` level to completely remove scrollbars on desktop devices, while maintaining `overflow-y: auto` at the `breakpoint-large` level for tablets. This design decision relies on content managers controlling bio text length to prevent overflow on desktop, ensuring a clean scrollbar-free experience on larger screens.
📚 Learning: 2025-07-02T08:15:36.173Z
Learnt from: yuramax
PR: speedandfunction/website#191
File: website/modules/asset/ui/src/scss/_leadership-team.scss:213-216
Timestamp: 2025-07-02T08:15:36.173Z
Learning: In website/modules/asset/ui/src/scss/_leadership-team.scss, the `.leader-bio` element intentionally uses `overflow-y: hidden` at the `breakpoint-extra-large` level to completely remove scrollbars on desktop devices, while maintaining `overflow-y: auto` at the `breakpoint-large` level for tablets. This design decision relies on content managers controlling bio text length to prevent overflow on desktop, ensuring a clean scrollbar-free experience on larger screens.
Applied to files:
website/modules/asset/ui/src/scss/_testimonials.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: security-scan
- GitHub Check: unit-tests
- GitHub Check: lint
🔇 Additional comments (1)
website/modules/asset/ui/src/scss/_testimonials.scss (1)
75-86: Mobile-first scrollable content looks solid.Good switch away from truncate-lines. The vertical-only scroll with wrapping, iOS momentum, and stable scrollbar gutter addresses the core issue on small screens.
…ndfunction/website into 767-fix/long-testimonials-are-cut
|
3 participants
Uh oh!
There was an error while loading. Please reload this page.