-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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] Show the JSX by default for small examples #17831
Conversation
Details of bundle changes.Comparing: 5c70cfc...9a13d1e
|
0b59000
to
5c231b3
Compare
5c231b3
to
c0a1f88
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of displaying short demo code. Source discoverability is sometimes not as good as I'd like it to be.
This comment has been minimized.
This comment has been minimized.
02f7105
to
053980b
Compare
f0669c6
to
8853b97
Compare
8853b97
to
c67ebed
Compare
Yeah, was on my mental to-do list if everyone agreed with the basic premise of this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the idea 😍, I think that this proof of concept is promising! I have looked at almost (but not all, for instance, /customization) of the demos, the things I have noticed:
- https://deploy-preview-17831--material-ui.netlify.com/components/rating/#half-ratings, we can kill the parent div
- https://deploy-preview-17831--material-ui.netlify.com/components/container/ we should keep the source collapsed, move the hardcoded code preview underneath.
- https://deploy-preview-17831--material-ui.netlify.com/components/grid-list/#advanced-grid-list I'm wondering if 15 lines shouldn't the maximum allowed
- https://deploy-preview-17831--material-ui.netlify.com/components/checkboxes/#customized-checkbox Should we remove the tabulations (4 spaces in this context)?
- https://deploy-preview-17831--material-ui.netlify.com/components/paper/#paper I think that we can remove the wrapping div.
- https://deploy-preview-17831--material-ui.netlify.com/components/snackbars/#notistack I think that we should disable this preview
- https://deploy-preview-17831--material-ui.netlify.com/components/chips/#chip-array something is off with the regexp
- https://deploy-preview-17831--material-ui.netlify.com/components/modal/#simple-modal I think that we can move the "Click to get the full Modal experience!" in the markdown, outside the demo
- https://deploy-preview-17831--material-ui.netlify.com/components/textarea-autosize/#maximum-height maybe use
.repeat()
on the default value to get a shorter demo - https://deploy-preview-17831--material-ui.netlify.com/components/use-media-query/#using-material-uis-breakpoint-helpers Something is off. I think that we can stick to the previous version
- https://deploy-preview-17831--material-ui.netlify.com/components/skeleton/ I think that we should disable the previews
- https://deploy-preview-17831--material-ui.netlify.com/components/tooltips/#tooltips The Fab escapes the container (bottom right)
- https://deploy-preview-17831--material-ui.netlify.com/system/flexbox/ in the system demos, we might want to disable all the previews, and likely move the code example after the demo.
Thanks.
It should be fixed now. |
CircleCI is borked. |
@mbrookes I was eager/excited to try the new version on Netlify to be honest (so I helped a bit), I'm frustrated, we have a couple more issues to solve first 🙃 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still wonder if 20 lines is the right threshold, however, I really like the shape this effort is taking. It incentives us to write smaller and cleaner demos 👌.
- https://deploy-preview-17831--material-ui.netlify.com/components/progress/#circular-static We have an opportunity to make the demo easier to read and to copy and paste by using one class name on the root div vs x on each progress.
- https://deploy-preview-17831--material-ui.netlify.com/components/progress/#linear-indeterminate we could use CSS instead of
<br />
for the spacing.
8a60cae
to
0c74a0f
Compare
0c74a0f
to
e8df67a
Compare
It's arbitrary, but there are some demos that are straightforward, despite being longer, for example: We can always disable any that don't benefit from a preview. |
6cd4b7a
to
4919f28
Compare
bb08010
to
b9fa143
Compare
b9fa143
to
c93b61e
Compare
@mbrookes Great job! |
Related to #17483 (comment)
Before:
https://material-ui.com/components/avatars/
After:
https://deploy-preview-17831--material-ui.netlify.com/components/avatars/