[696] fix/button padding border - #188
Conversation
📝 WalkthroughWalkthroughThe update modifies button group styles in a SCSS file by adjusting padding, adding a border to grouped buttons, and removing the border from the last button in the group. A blank line was also inserted after a transition declaration, which does not impact functionality. 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/modules/asset/ui/src/scss/_buttons.scss (1)
77-79: Confirm padding token & consistencyPadding was changed from
12pxto11px, which breaks alignment with other button variants that still use the 12 px baseline. Unless the design system explicitly calls for an 11 px value here, consider keeping the tokenized12pxor introducing a variable (e.g.$btn-padding-mobile) to keep spacing consistent and easier to tweak globally.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/modules/asset/ui/src/scss/_buttons.scss(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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: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: 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.
website/modules/asset/ui/src/scss/_buttons.scss (4)
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: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.011Z
Learning: Applies to modules/asset/ui/src/index.js : modules/asset/ui/src/index.js: relaxed function lines and style rules
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.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: e2e-tests
- GitHub Check: security-scan
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
website/modules/asset/ui/src/scss/_buttons.scss (1)
74-80: Last child inherits new border; add explicit override to meet design goalYou’ve added a
border: 1px solid $gray-500to every.sf-buttoninside a.two-buttonsgroup, but the rule that should remove that border for the last button was not introduced. As a result, the last button will still display the border, which contradicts the stated PR goal and the summary.Proposed fix:
&:last-child { background: none; color: $gray-500; box-sizing: border-box; text-decoration: none; background-color: $white; + border: none; // avoid double-border/visual glitch }This keeps the new border for all but the trailing button and avoids a visual artifact on the right edge of the group.
Likely an incorrect or invalid review comment.
|



Uh oh!
There was an error while loading. Please reload this page.