Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideStandardizes underline styling for links and buttons and refactors the external-link icon implementation to use a dedicated mixin and static SVG asset with improved alignment. Class diagram for updated SCSS mixins and link stylesclassDiagram
direction LR
class text_icon {
<<mixin>>
+display inline-flex
+gap $gap
+align-items center
+&::$position width $size
+&::$position min-width $size
+&::$position height $size
+&::$position content ""
+&::$position background-color $color
+&::$position mask-image url(../img/static/#icon.svg)
+&::$position mask-position center
+&::$position mask-repeat no-repeat
+&::$position transition background-color .5s $ease-out-expo
}
class text_external_icon {
<<mixin>>
+display inline-block
+gap 0
+&::$position display inline-block
+&::$position margin-inline-start $gap
+&::$position vertical-align middle
}
class a_base {
<<selector a>>
+text-decoration none
+color $link-color
+cursor pointer
+hover underline
}
class a_target_blank {
<<selector a[target=_blank]>>
+@include text_external_icon(external)
}
class a_button_shared {
<<selector a,button>>
+text-decoration-thickness 1px !important
+text-underline-position from-font
+text-decoration-skip-ink none
+cursor pointer
}
class link_external_class {
<<selector .link-external>>
+@include text_external_icon(external)
}
text_external_icon ..|> text_icon : uses
a_target_blank ..> text_external_icon : includes
link_external_class ..> text_external_icon : includes
a_base <|-- a_target_blank
a_button_shared ..> a_base : shares underline styling
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The global
a, buttonrule withtext-decoration-thickness: 1px !importantandtext-underline-positionmay unintentionally affect all buttons and any custom link/button variants; consider scoping these underline styles to specific link/button classes or contexts instead of all anchors and buttons. - You now set
cursor: pointerboth in thearule and in the newa, buttonblock; consider consolidating this to a single place to avoid redundant declarations and make overrides easier to reason about. - Changing
text-iconto use a file-basedmask-image: url(../img/static/#{$icon}.svg)while leaving the SVG map function in place for other icons splits the icon-loading strategy; consider either consistently using the helper for all icons or clearly separating which mixins use inline SVG vs. file assets to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The global `a, button` rule with `text-decoration-thickness: 1px !important` and `text-underline-position` may unintentionally affect all buttons and any custom link/button variants; consider scoping these underline styles to specific link/button classes or contexts instead of all anchors and buttons.
- You now set `cursor: pointer` both in the `a` rule and in the new `a, button` block; consider consolidating this to a single place to avoid redundant declarations and make overrides easier to reason about.
- Changing `text-icon` to use a file-based `mask-image: url(../img/static/#{$icon}.svg)` while leaving the SVG map function in place for other icons splits the icon-loading strategy; consider either consistently using the helper for all icons or clearly separating which mixins use inline SVG vs. file assets to avoid confusion.
## Individual Comments
### Comment 1
<location path="src/scss/03-base/_links.scss" line_range="18-19" />
<code_context>
}
}
+a,
+button {
+ text-decoration-thickness: 1px !important;
+ text-underline-position: from-font;
</code_context>
<issue_to_address>
**issue (bug_risk):** Selector couples anchor and button styles in a way that may cause unintended button underlines and overrides.
This shared `a, button` rule forces `text-decoration` and `cursor: pointer` onto all buttons, which can add unwanted underlines or affect decoration on icon/ghost/unstyled buttons, and also duplicates the existing `cursor: pointer` on `a`. Please narrow the selector (e.g., to link-style buttons or a specific class) or apply these styles only to anchors if the change is link-specific.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| */ | ||
|
|
||
| @mixin text-icon($icon: "arrow", $position: after, $color: currentColor, $size: 12px, $gap: 8px) { | ||
| position: relative; |
There was a problem hiding this comment.
Default icon parameter references non-existent SVG file
Medium Severity
The text-icon mixin's default $icon parameter is "arrow", but the mask-image now resolves to url(../img/static/arrow.svg) — and no arrow.svg exists in src/img/static/. Only external.svg is present there. Previously this default worked because it was looked up via the inline get-svg-url map which contained "arrow". Any future caller using text-icon() without arguments will get a broken icon.
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
1 participant
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.
Fix des liens par défault sur le soulignements qu'on retrouve toujours de cette manière de configuré dans les figma
fix du positionnement de l'icône des liens externes quand le texte fais plusieurs lignes
Summary by Sourcery
Adjust default link styling and external link icon handling for consistency with design specifications.
Enhancements:
Note
Low Risk
Low risk CSS-only changes: updates link/button underline defaults and tweaks external-link icon rendering/positioning; main risk is minor visual regression across browsers.
Overview
Fixes default link underline rendering to better match design specs by standardizing underline thickness/positioning and disabling ink-skipping for
aandbutton.Refactors the external-link icon styling: switches the icon mask from
get-svg-url()(inlined SVG map) to a dedicated staticexternal.svg, and adjusts thetext-*iconmixins (min-width,mask-size: contain, inline/vertical alignment) to keep the icon aligned when link text wraps.Written by Cursor Bugbot for commit 48f5a82. This will update automatically on new commits. Configure here.