Collapsible content media with global settings #1232
Conversation
media with global settings
There was a problem hiding this comment.
Thank you Ludo!
Observation
- Shadow vertical offset negative would overlap other media, but not when the value is positive and not if it is a section
- Just a note for me to check where we landed for the shadow potentially hiding titles, labels, etc. cc. @nicklepine
Otherwise looks good!
@melissaperreault I believe it's known and technically logic for the shadow of an element to be hover what precedes them. It's the natural "stacking" context. Think of it as a stair case. The first step from the bottom being the first section, then the second is above it and so on. I think the way to go about it is just to trust merchants to add enough section spacing and section padding to take into account the shadows they're adding 🤔 We could have it always be over the section above by making sure we apply the |
assets/collapsible-content.css
Outdated
| @@ -10,8 +10,9 @@ | |||
| } | |||
| } | |||
|
|
|||
| .collapsible-content__media { | |||
| background-color: transparent; | |||
| .collapsible-content { | |||
There was a problem hiding this comment.
Since we created the class .isolate in this PR can we re-use it here? #1247
It does the same thing 🤔 What do you think?
There was a problem hiding this comment.
Ill add the class where you added it too so that when merging there isn't any conflict with your PR :)
There was a problem hiding this comment.
Looks great! 🎉 You may just need to rebase since my PR was merged #1247
Why are these changes introduced?
Tackles part of #1043
What approach did you take?
Added the
global-media-settingsclass on the media wrapper and applied on the section, this styling:So that the shadow could come up. Same fix is applied on other sections and there is a PR looking into refactoring that approach: #1209
To test
Go to the editor and play around with the global media settings
Demo links
Checklist