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

[docs-infra] Adjust demo container to be glued to the toolbar #37744

Merged
merged 8 commits into from
Jul 3, 2023

Conversation

danilo-leal
Copy link
Contributor

@danilo-leal danilo-leal commented Jun 27, 2023

Opening the PR to get a sense of how this feels. It's an exploration coming from @michaldudak's feedback that the code block feels disconnected now that the demo toolbar is part of the demo container itself. These changes make sense to me, but for some reason, I'm not 100% sure. Though definitely see how it can fly!

https://deploy-preview-37744--material-ui.netlify.app/joy-ui/react-button/#disabled

@danilo-leal danilo-leal added design This is about UI or UX design, please involve a designer scope: docs-infra Specific to the docs-infra product labels Jun 27, 2023
@danilo-leal danilo-leal self-assigned this Jun 27, 2023
@michaldudak
Copy link
Member

Nice, I like it more than the live version.

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

👍

docs/src/modules/components/DemoToolbar.js Outdated Show resolved Hide resolved
Signed-off-by: Olivier Tassinari <olivier.tassinari@gmail.com>
@mui-bot
Copy link

mui-bot commented Jun 30, 2023

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against fb6573e

@danilo-leal danilo-leal marked this pull request as ready for review June 30, 2023 14:49
@alexfauquette
Copy link
Member

I was even wondering why not putting the code between the demo and action bar

image

@danilo-leal
Copy link
Contributor Author

danilo-leal commented Jun 30, 2023

@alexfauquette that's not as ideal because lots of these code blocks ⎯ if not all ⎯ are expandable by clicking on the <> icon there. So, by having the toolbar at the bottom, the control to expand & collapse the code block would be going up & down, which is not great 😅 Also, I think the visual hierarchy (both visually and from the information standpoint) makes a bit more sense with Demo > Toolbar > Code as the toolbar doesn't necessarily affect the code snippet below, just what's up there in the demo container.

@alexfauquette
Copy link
Member

alexfauquette commented Jun 30, 2023

the toolbar doesn't necessarily affect the code snippet below

there is only 2 buttons that so not affect the code (focus and reset)

My main concern was that the toolbar is in between so it feel a bit weird since usually toolbar are at the top/bottom of what they control. But yes putting it at the bottom has also serious issues

@danilo-leal
Copy link
Contributor Author

@alexfauquette gotcha. Either way, we could continue to reflect on this outside the scope of this PR 😬

@oliviertassinari
Copy link
Member

I have pushed two commits to integrate #37693 into this PR.

@danilo-leal
Copy link
Contributor Author

danilo-leal commented Jun 30, 2023

@alexfauquette & @oliviertassinari need an approval now if we're feeling good about it! 😬

@danilo-leal
Copy link
Contributor Author

For the sake of benchmarking: Ionic places the demo toolbar on the top of the demo container. I think it could make sense as well, however, I'd be slightly worried about this somewhat big change?! Wouldn't do it right now but just sharing it, I guess.

Screen Shot 2023-06-30 at 18 33 32

@michaldudak michaldudak merged commit 28d1cc2 into master Jul 3, 2023
24 checks passed
@michaldudak michaldudak deleted the demo-container-tweaks branch July 3, 2023 07:12
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 scope: docs-infra Specific to the docs-infra product
Projects
Status: Recently completed
Development

Successfully merging this pull request may close these issues.

None yet

5 participants