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] Improve demo container and related components design #41827

Merged
merged 7 commits into from Apr 11, 2024

Conversation

danilo-leal
Copy link
Contributor

@danilo-leal danilo-leal commented Apr 9, 2024

This PR primarily introduced a few color and border-related improvements. However, there's a functional change here that I want to highlight so reviewers can give me feedback: I've hidden the code copy button when a code block is within the demo container. That's because the demo toolbar already offers a copy button, and I've always found it to be a bit confusing to have both buttons with the same icon that seemingly does the same thing (unless that's not true).

So, in summary, the code block copy button will only appear if the code block is outside of a Demo container.

https://deploy-preview-41827--material-ui.netlify.app/experiments/docs/demos/

@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 Apr 9, 2024
@danilo-leal danilo-leal self-assigned this Apr 9, 2024
@mui-bot
Copy link

mui-bot commented Apr 9, 2024

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against c74ba19

@zanivan
Copy link
Contributor

zanivan commented Apr 9, 2024

I think we can remove the copy button when it's redundant. Analyzing the GA data for the last 30 days, most of the users (~85%) choose to copy the content using the toolbar icon (copy).

Screenshot 2024-04-09 at 17 45 16

The copy-click stands for the code copy button in the code block.

@bharatkashyap
Copy link
Member

This PR primarily introduced a few color and border-related improvements. However, there's a functional change here that I want to highlight so reviewers can give me feedback: I've hidden the code copy button when a code block is within the demo container. That's because the demo toolbar already offers a copy button, and I've always found it to be a bit confusing to have both buttons with the same icon that seemingly does the same thing (unless that's not true).

So, in summary, the code block copy button will only appear if the code block is outside of a Demo container.

https://deploy-preview-41827--material-ui.netlify.app/experiments/docs/demos/

The only additional use case to consider is with multi-tab demos, the per-tab copy button lets you copy the content within a tab, whereas the copy button in the toolbar concatenates content in all tabs and copies it. If that's a low-enough value use case to lose, I see no other reason not to do this 👍🏻

docs/src/modules/components/Demo.js Outdated Show resolved Hide resolved
docs/src/modules/components/DemoEditor.tsx Outdated Show resolved Hide resolved
@alexfauquette
Copy link
Member

About the use case of multiple tabs demo, we could bring back this button for multiple files demo only if it feel missing

@danilo-leal
Copy link
Contributor Author

the per-tab copy button lets you copy the content within a tab, whereas the copy button in the toolbar concatenates content in all tabs and copies it.

Oh, that's interesting! I think the ability to copy the code just from that tab or the entire code spanning multiple tabs is cool, and we should support it. I'm just not sure about the UX with the code block copy button doing the former and the toolbar copy button doing the latter. What bothers me is that they're visually the same, and there's no expectation set beforehand for the user.

A solution that crossed my mind, and we can potentially explore this on @bharatkashyap's multi-tab demo PR, is that when that's the case, the copy button on the demo toolbar becomes a menu trigger that displays the two options: "Copy code from this tab" and "Copy code from all tabs". How does this sound?

@danilo-leal danilo-leal merged commit 47b624d into mui:next Apr 11, 2024
22 checks passed
@danilo-leal danilo-leal deleted the demo-container-improvements branch April 11, 2024 11:00
DiegoAndai pushed a commit to DiegoAndai/material-ui that referenced this pull request Apr 16, 2024
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
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants