[717] The vertical alignment of the photos with the grey separators is slightly off - #204
Merged
killev merged 4 commits intoJul 4, 2025
Conversation
…h-the-grey-separators-is-slightly-off
Contributor
📝 WalkthroughWalkthroughThe SCSS for the leadership team UI was refactored, focusing on simplifying and adjusting border and hover styles for Changes
Suggested reviewers
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
website/modules/asset/ui/src/scss/_leadership-team.scss (2)
21-37: Duplicate declarations – can be DRYed up
border-top: noneand.leader-header { margin-bottom: 1px; }are repeated for both the normal and:hoverstates inside the same media query. One rule block is enough – the second adds no visual difference and slightly increases CSS size.& + .leadership-list { .leader-card { @include breakpoint-large { - border-top: none; - .leader-header { margin-bottom: 1px; } + border-top: none; + .leader-header { margin-bottom: 1px; } } - &:hover { - @include breakpoint-large { - border-top: none; - .leader-header { margin-bottom: 1px; } - } - } + &:hover { @include breakpoint-large { border-top: none; } } } }
180-186: Redundant small-viewport overrideInside
&:hoveryou first setborder-top: 1px solid transparent(mobile) and then immediately override it for large screens. The transparent value is identical to the non-hover state, so the mobile rule is effectively a no-op and can be dropped.&:hover { - border-top: 1px solid transparent; - @include breakpoint-large { - border-top: 1px solid $whisper; + border-top: 1px solid $whisper; } }Removing it keeps the intent clear and trims a few bytes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/modules/asset/ui/src/scss/_leadership-team.scss(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 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.
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.152Z
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: 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.
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.
Learnt from: yuramax
PR: speedandfunction/website#121
File: website/modules/asset/ui/src/scss/_vacancies.scss:81-89
Timestamp: 2025-05-26T15:54:06.220Z
Learning: In CSS, when evaluating cascade conflicts, always check specificity carefully. A nested sibling selector like `.vacancy-card + .sf-vacancies_footer h2` has higher specificity than a general descendant selector like `.sf-vacancies_footer h2`, so the more specific rule will take precedence and won't be overridden by the less specific one.
Learnt from: CR
PR: speedandfunction/website#0
File: .cursor/rules/code-conventions.mdc:0-0
Timestamp: 2025-07-01T07:44:33.034Z
Learning: Applies to modules/asset/ui/src/index.js : modules/asset/ui/src/index.js: relaxed function lines and style rules
website/modules/asset/ui/src/scss/_leadership-team.scss (5)
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.152Z
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: 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.
Learnt from: CR
PR: speedandfunction/website#0
File: .cursor/rules/code-conventions.mdc:0-0
Timestamp: 2025-07-01T07:44:33.034Z
Learning: Applies to modules/asset/ui/src/index.js : modules/asset/ui/src/index.js: relaxed function lines and style rules
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.
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.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: security-scan
- GitHub Check: e2e-tests
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
website/modules/asset/ui/src/scss/_leadership-team.scss (1)
51-54: Nice touch – transparent top border prevents layout shift on hoverAdding a transparent 1 px top border keeps the card height constant, eliminating the flicker you used to get when the real border appears on hover. 👍
🔍 Vulnerabilities of
|
| digest | sha256:ae498fc6349b3c03f6e45f11dc64fee90f2caa251b673b5e80ea3dfae602b198 |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 290 MB |
| packages | 977 |
📦 Base Image node:23-alpine
| also known as |
|
| digest | sha256:b9d38d589853406ff0d4364f21969840c3e0397087643aef8eede40edbb6c7cd |
| vulnerabilities |    |
Description
| ||||||||||||
Description
| ||||||||||||
Description
|
…h-the-grey-separators-is-slightly-off
killev
enabled auto-merge (squash)
July 4, 2025 20:19
…h-the-grey-separators-is-slightly-off
killev
approved these changes
Jul 4, 2025
|
killev
deleted the
717-the-vertical-alignment-of-the-photos-with-the-grey-separators-is-slightly-off
branch
July 4, 2025 20:30
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
4 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.
Fixes the vertical alignment issue with photos and grey separators. Adjusted styles to avoid layout shift and border behavior on hover.