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

[website] Fix pricing table style #38681

Merged
merged 7 commits into from
Aug 29, 2023

Conversation

alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Aug 28, 2023

Fix the left overflow of nested rows

Before After
image image

@alexfauquette alexfauquette added design This is about UI or UX design, please involve a designer website Pages that are not documentation-related, marketing-focused. labels Aug 28, 2023
@mui-bot
Copy link

mui-bot commented Aug 28, 2023

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 1420873

Copy link
Contributor

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks for doing it! How about we also take advantage of the PR to fix the inside of the accordion row hover?

Screen Shot 2023-08-28 at 09 53 38

Comment on lines +1165 to +1178
<Collapse
in={dataGridCollapsed}
timeout={700}
sx={(theme) => ({
position: 'relative',
ml: 1.5,
borderLeftWidth: '2px',
borderLeftStyle: 'solid',
borderColor: 'grey.100',
...theme.applyDarkStyles({
borderColor: 'primaryDark.700',
}),
})}
>
Copy link
Member Author

@alexfauquette alexfauquette Aug 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the main modification.

For a reason I ignore, all the nested element were nested thanks to some margin, and the left bar were made thanks to a box with absolute position.

I replaced all that with a margin-left and border-left on the Collapse item, such that all the children are translated to the right. Which allows to remove props nested everywhere.

I did not found issue when resizing the window, including with smartphone debugger
@siriwatknp do you have a concern about this change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad thing about this approach, it seems, is that the rows inside the accordion are now misaligned with the rest of the columns/rows:

Screen Shot 2023-08-28 at 19 08 53

@danilo-leal
Copy link
Contributor

Shoot, the column alignment is now perfect but the row hover started to leak out again 😅

Screen Shot 2023-08-29 at 08 30 47

Copy link
Contributor

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me! Thanks for this one! 🤙

docs/src/components/pricing/PricingTable.tsx Outdated Show resolved Hide resolved
Co-authored-by: Danilo Leal <[email protected]>
Signed-off-by: Alexandre Fauquette <[email protected]>
@alexfauquette alexfauquette merged commit ffab89c into mui:master Aug 29, 2023
@alexfauquette alexfauquette deleted the fix-pricing-style branch August 29, 2023 13:26
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Sep 8, 2023
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design This is about UI or UX design, please involve a designer website Pages that are not documentation-related, marketing-focused.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants