-
Notifications
You must be signed in to change notification settings - Fork 934
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
External links: fix Safari display bug. #4439
Conversation
Prevents icon distortion in some browsers, like Firefox.
| #{$width} #{$height}; | ||
| $image-props: url("#{$path}/#{$filename-ie11}") no-repeat center / #{$width} #{$height}; | ||
| contain; | ||
| $image-props: url("#{$path}/#{$filename-ie11}") no-repeat center / contain; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the $width and $height variables unused now after these changes? (i.e. can they be removed?)
Oddly, there's also an assignment a few lines below this line, which I'm not sure would have had any effect? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$width and $height are used to get consistent sizing of the icons across all browsers in both background and mask.
It seemed more straightforward and allowed consistent rendering this way. But open to any improvements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohhh, my brain misread line 134 as $width: (variable assignment) instead of width: (CSS property). Disregard 🤦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good! Just getting back from break so the brain fog on this was real heavy. 😅
|
Thank you! |
Previews
External links →
Description
Improved display of External links. Now external links display consistently on all browsers. Specifically fixes a visual regression in Safari when using mask to set the icon. Fixes #4434.
Additional information
Previously the external-link mixin was using
display: inlineon the::afterpseudoelement to prevent the icon from breaking into it's own line on narrow screen sizes #3395. This causes issues in Safari when using masks because the background set doesn't get clipped properly.Setting this to
display: inline-blockfixes the display issue in Safari, but then breaks the flow of the icon on smaller screens.Example from designsystem.digital.gov
The fix
I'm using
position: absoluteto properly place the icon and using inline-block to get it to display properly in Safari.Before you hit Submit, make sure you’ve done whichever of these applies to you:
I've also removed the dimensions being set via
background-sizeandmask-sizeinadd-color-icon(). This was causing the external link icon to appear squished at certain sizes.This change affects all components that use
add-color-icon()(_alerts.scss:26for example).npm testand make sure the tests for the files you have changed have passed.