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

[Stack] Fix spacing when there are <style> children #34966

Merged
merged 1 commit into from
Jul 1, 2023

Conversation

cmd-johnson
Copy link
Contributor

@cmd-johnson cmd-johnson commented Oct 31, 2022

Fix #34965.

And looking at the CSS used by the <Stack> component, it looks like the issue is the way the children that the margin is applied to are selected: & > :not(style) + :not(style). This excludes any children that have a <style> element in front of them.

By swapping the + (adjacent sibling) combinator for the ~ (general sibling) combinator, this issue should be fixed without affecting anything else.

@mui-bot
Copy link

mui-bot commented Oct 31, 2022

Netlify deploy preview

https://deploy-preview-34966--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 95c3b48

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 31, 2022

I think that it would be great to fix the other instance in the codebase where we make this mistake. My bad, I added the logic in the case that developers would not follow the correct Emotion SSR setup, but I didn't test that it would actually work. Soon (browser support), we will be able to use flex gap instead of this trick.

@zannager zannager added the component: Stack The React component. label Nov 1, 2022
@cmd-johnson
Copy link
Contributor Author

Those are the only occurrences outside of docs/ where I could find :not(style). Of those matches in docs/, most just use & > :not(style) and shouldn't be affected by this.
Other than that, there's DividerText, Links, UnderlineLinks and ToggleButtonSizes, which use either a Box with a custom sx style or a styled('div'). If you want I could replace all those usages with Stack instead and it should all work just the same.

Also, when you talk about "not following the correct Emotion SSR setup", what do you mean exactly?
I'm using React 18's renderToPipeableStream and Remix, with basically the same SSR rendering logic as this: https://github.com/jacob-ebey/remix-chakra/blob/main/app/entry.server.tsx

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 24, 2023
@oliviertassinari oliviertassinari changed the title [Stack] Fix spacing when there are <style> children (#34965) [Stack] Fix spacing when there are <style> children Jul 1, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 1, 2023
@oliviertassinari oliviertassinari added PR: out-of-date The pull request has merge conflicts and can't be merged enhancement This is not a bug, nor a new feature labels Jul 1, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 1, 2023
@oliviertassinari oliviertassinari merged commit 3a471fa into mui:master Jul 1, 2023
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Stack The React component. enhancement This is not a bug, nor a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Stack] doesn't space children correctly when there are style tags between them
4 participants