-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Global Styles Sidebar: wrap the preview box in the CardMedia component #39980
Conversation
CardMedia
component
Size Change: +8 B (0%) Total Size: 1.22 MB
ℹ️ View Unchanged
|
After: On its own, this is a nice improvement. However there's a small problem — the border radius is correct in the code, 2px, however that is meant to be the box radius or the "inner" radius. When the border or shadow is outset, then the outermost radius becomes border-radius+border-width, i.e. in this case, 3px. The outermost radius should be 2px, to compare with the other elements: One solution is to account for that and subtract 1px in the border radius. That makes it look like this: When that happens if you look close enough, the inner background of the card itself cuts into the border. If in the inspector I apply |
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.
If I understand correctly, I think Joen's point about the border radius inaccuracy is a separate issue that we can perhaps compensate for at the Card
level.
Thank you @mirka and @jasmussen ! I've opened a follow-up PR #40032 to tweaks the border-radius size at the component level. |
What?
This PR aims at improving the way the border around the preview box in the Global Styles sidebar is rendered, as flagged in #38934 under the "Smooth the corners of the styles box." point.
Why?
This is in order to follow the design specs highlighted in #38934
How?
Until now, the
<StylesPreview />
component has been a direct child of<Card />
. Since the border on theCard
component is implemented viabox-shadow
(see #35245 for some conversations around that), the<StylesPreview />
component has been rendering on top of the rounded corners of the<Card />
component, causing those rounded corners to look glitchy.This PR avoids the issue by wrapping
<StylesPreview />
in a<CardMedia />
component. The<CardMedia />
component, in fact, is able to apply the same corner radius to itself and therefore, with the addition of theoverflow: hidden
rule, is able to fix the issue described in the paragraph above.Note: as explained in #35245 and #38934, it's still possible that browser rendering engines can introduce some sub-pixel / anti-aliasing artefacts, especially with the technique that is currently employed by the
Card
component for rendering its border via thebox-shadow
property.Testing Instructions
On
trunk
:Apply the changes from this PR, and this time notice how the borders of the preview box are being rendered in a "smoother" way, without being overlapped by the contents of the box itself.
(Note: this fix may be difficult to appreciate depending on the resolution of the screen used to review them. One way to make the fix more noticeable is to change the value of the
cardBorderRadius
option to a higher value, e.g.6px
)Screenshots or screencast
With a higher value for
border-radius
(to demo/highlight the fix better):