Clear rte page content #128
Conversation
assets/section-main-product.css
Outdated
| .product__accordion .accordion__content:after { | ||
| clear: both; | ||
| content: ""; | ||
| display: block; | ||
| } |
There was a problem hiding this comment.
I think you're right in your description. It would need to be added for the .rte class I think. Cause if you go on the home page or anywhere else you can add a section, then add a page section and use page with floated image then you will get the same issue:
There was a problem hiding this comment.
True, that will come up more often then. Moved the same fix to the rte class now. I can't think of a reason why this wouldn't be ok, but maybe there's a rte case out there I'm not thinking of. Curious if anyone thinks there's a safer way to handle this.
There was a problem hiding this comment.
I guess we could have a rte-page specific class but I don't think having it to the more general rte one should create any problems. It's adding content in places where it might not be needed so maybe we should avoid it, to keep things clean 🤔 what do you think ?
I think there are basically three places where it might be needed, the main page, the page section and on the product page for the collapsible tab and for the popup which both have a page picker.
There was a problem hiding this comment.
I think it should be fine. Since the content is empty, it'll just be a 0 height pseudo-element on the page.
There was a problem hiding this comment.
Also I haven't seen a clearfix in a long time. This brings me back!
There was a problem hiding this comment.
Yeah I'm ok with the content as is. I'll leave it simple as is if you don't have a strong objection Ludo
kmeleta
force-pushed
the
tab-page-content-fix
branch
from
0d15446
to
8fa3d25
Compare
kmeleta
force-pushed
the
tab-page-content-fix
branch
from
8fa3d25
to
68bb5ec
Compare
There was a problem hiding this comment.
Works well. Tested on:
- Product page: page content
- Page section
- Page template
On Chrome/Firefox/Safari, no issues.
I don't think this will break in the future, since we're not really using float anymore unless it's RTE or an app is using it, but that's a separate topic.
Co-authored-by: Chris Berthe <chris.berthe@shopify.com>
* clear collapsible tab content * apply clear fix to rte class * Update single quote Co-authored-by: Chris Berthe <chris.berthe@shopify.com> Co-authored-by: Chris Berthe <chris.berthe@shopify.com>
Why are these changes introduced?
Fixes #2
What approach did you take?
Added a clearfix (via
:after) to the rte class. Right aligned media in the page content rte getsfloat: right. If that content isn't cleared by something else in that page, the content can bleed out of its container and/or cause other layout issues (i.e. collapsible tabs on the product page).Other considerations
The original issue was scoped for page content within collapsible tabs on product pages, but really this same sort of issue could apply anywhere RTE page content can be displayed.
Demo links
Checklist