Skip to content
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

Reverse margin direction for :before and :after to prevent margin-collapse? #26

Closed
miragecraft opened this issue Sep 2, 2020 · 11 comments · Fixed by #33
Closed

Reverse margin direction for :before and :after to prevent margin-collapse? #26

miragecraft opened this issue Sep 2, 2020 · 11 comments · Fixed by #33

Comments

@miragecraft
Copy link

Just wondering what is the rationale to use margin-top for :before and margin-bottom for :after instead of the other way around.

Seems to me if you reverse margin direction you avoid having to define padding: 0.05px 0; to prevent margin collapse.

@michaeltaranto
Copy link
Contributor

Hey 👋

Thanks for the suggestion, you had me for a minute there thinking I had overlooked a much simpler alternative! But unfortunately the approach you described does not work as intended — it trims the height of the text node, but the visual space is still left above and below the node from a container perspective.

Here is a demo of the margin collapse guard working in Playroom.

@miragecraft
Copy link
Author

I found the cause - empty elements will have their top and bottom margin collapse, which is what happened here with the :before and :after elements.

It didn't matter that the margin direction were reversed, they collapsed within themselves first and then was able to collapse with parent's margin.

Sorry for the false alarm.

@miragecraft
Copy link
Author

miragecraft commented Sep 3, 2020

Just discovered something, instead of setting the :before and :after to display: block, setting them to flex or table (wider browser support) will allow you to drop the margin collapse guard.

This needs to be combined with reversed margin direction suggested earlier (that didn't work by themselves).

Here's a demo.

@miragecraft miragecraft reopened this Sep 3, 2020
@michaeltaranto
Copy link
Contributor

Hey now that's interesting! I remember coming across table having special powers to prevent collapse but forgot about it here. What's interesting too is we can also drop the height: 0 on the pseudo elements — that was required to prevent the collapsing of an empty block element, but seemingly not for a table.

So far this is looking promising.

{
  "fontSize": "",
  "lineHeight": "",
- "padding": "0.05px 0",
  "::before": {
    "content": "''",
    "marginBottom": "",
+   "display": "table"
-   "display": "block",
-   "height: 0"
  },
  "::after": {
    "content": "''",
    "marginTop": "",
+   "display": "table"
-   "display": "block",
-   "height: 0"
  }
}

@phloe
Copy link

phloe commented Sep 4, 2020

As a sidenote the CSS custom props based text-crop I have been working on uses flipped margins - but with padding moved to the pseudos to prevent collapse :)

https://jsfiddle.net/phloe/yrg3L8to/

But just using display: table; is much cleaner :)

@adamduncan
Copy link

adamduncan commented Feb 14, 2021

I was digging into a similar alternative to prevent collapsing margins and the need for padding elements.

Josh Comeau has a great post on margin collapsing. It sparked a thought: Could one use display: flow-root on the text element? That would also nicely negate the need for factoring padding into the calculation:

https://www.joshwcomeau.com/css/rules-of-margin-collapse/ (under "Margin-blocked")

@phloe
Copy link

phloe commented Feb 14, 2021

Yeah - much cleaner. iOS only supports it from 13 and up though; https://caniuse.com/flow-root

@pdimova
Copy link

pdimova commented Dec 3, 2021

I was digging into a similar alternative to prevent collapsing margins and the need for padding elements.

Josh Comeau has a great post on margin collapsing. It sparked a thought: Could one use display: flow-root on the text element? That would also nicely negate the need for factoring padding into the calculation:

https://www.joshwcomeau.com/css/rules-of-margin-collapse/ (under "Margin-blocked")

This is it!

In our case, we had a misalignment issue displaying cropped link text inside a paragraph, so changing the pseudo-elements display values to flow-root instead of table solve it.

@michaeltaranto
Copy link
Contributor

@pdimova would you mind sharing a before and after? A codepen or codesandbox that shows the issue would be great.

Unfortunately a solution that is >=iOS 13 is not usable for us, but keen to see the problem you're facing.

@Vanals
Copy link

Vanals commented Dec 3, 2021

Hi @michaeltaranto ! I work with @pdimova and trying to solve with her our issue.

This is a code pen with our scenario:
https://codepen.io/pollx/pen/mdBJZoq

Seems like that using display: flow-root rather than display: table, in the .flex-child ::before and ::after, could solve our issue. Would be great if you could share your thoughts on this!

I am aware the HTML structure is not the cleanest one, but this is how it is atm :D.

...Unfortunately a solution that is >=iOS 13...
Do you mean this solution won't work for a user on < IOS 13? Even if using the supported browsers?
Safari IOS seems to be ok with that: https://caniuse.com/flow-root

Thank you.

@pdimova
Copy link

pdimova commented Dec 7, 2021

@pdimova would you mind sharing a before and after? A codepen or codesandbox that shows the issue would be great.

Unfortunately a solution that is >=iOS 13 is not usable for us, but keen to see the problem you're facing.

hey @michaeltaranto! Thank you for your reply!

I see my colleague has already shared a codepen example with you :)

Could you please elaborate more on this line here?

Unfortunately a solution that is >=iOS 13 is not usable for us, but keen to see the problem you're facing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants