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] Redesign on markdown page #27860

Merged
merged 33 commits into from
Aug 24, 2021

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova commented Aug 20, 2021

Preview: https://deploy-preview-27860--material-ui.netlify.app/components/alert/

Changes done:

Table content:

Prev:

Now:

Demo toolbar

Prev:

Now:

Page heading

Prev:

Now:

TODO:

API link

Prev:

Now:

Updated the colors on the content of the docs pages

Next iteration:

  • Sidebar

Scrollbar dark mode - will be handled after #25016 is fixed

@mui-pr-bot
Copy link

mui-pr-bot commented Aug 20, 2021

No bundle size changes (experimental)

Generated by 🚫 dangerJS against 5f3a240

@mnajdova
Copy link
Member Author

I noticed one problem when I played with the scrollbar, namely the global styles ar propagated to the demos too (see the scrollbar on the demo) Not sure what we should do with this one. cc @siriwatknp @oliviertassinari

image

I feel like in the end we may need to isolate all examples in iframes :\

@danilo-leal
Copy link
Contributor

Nice work! I've pushed some small changes here and there. Listing some other ones I couldn't do:

  1. The texts and the demo/code containers are slightly misaligned horizontally. Couldn't find where to play with this...

Screen Shot 2021-08-20 at 12 44 36

  1. There are some cases where the demo and code container don't have border-radius applied. No idea why...

Screen Shot 2021-08-20 at 12 39 17

  1. Couldn't find where to modify the colors of the filled and outlined demo container on dark mode.

Screen Shot 2021-08-20 at 12 56 42

  1. Couldn't find where to modify the max-width of the container that holds all the content but I played with some values directly on the inspector. This is something I personally think it makes for a much better reading experience (less horizontal movement of the eye) and also for a more overall balanced look since we gain negative space between the sidebar and the table of contents.

Screen Shot 2021-08-20 at 12 58 44

These values are just me playing, not necessarily the correct ones. Probably somewhere around 700px - 800px lies a good max-width. For reference, most of our users check the website in a 1920x1080 screen res which renders the div with 1152 width, which is too much 😅 But we can iterate on it on another PR!

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 20, 2021

see the scrollbar on the demo

@mnajdova There is an open GitHub issue that describes the next step for the scrollbar on dark mode. The plan is to remove the global overrides because it comes with too many downsides vs. the alternative.

@mnajdova
Copy link
Member Author

@mnajdova There is an open GitHub issue that describes the next step for the scrollbar on dark mode. The plan is to remove the global overrides because it comes with too many downsides vs. the alternative.

I've found it, linking it here for future reference #25016 I will revert for the now the changes for the scrollbar, we can come back to this after the v5 stable release.

@mnajdova mnajdova marked this pull request as ready for review August 23, 2021 15:02
@mnajdova mnajdova changed the title [WIP][docs] Redesign on markdown page [docs] Redesign on markdown page Aug 23, 2021
@mnajdova
Copy link
Member Author

These values are just me playing, not necessarily the correct ones. Probably somewhere around 700px - 800px lies a good max-width. For reference, most of our users check the website in a 1920x1080 screen res which renders the div with 1152 width, which is too much 😅 But we can iterate on it on another PR!

Agree, let's keep the scope smaller, so that we can iterate faster :)

@mnajdova mnajdova requested a review from siriwatknp August 23, 2021 15:04
@mnajdova
Copy link
Member Author

  1. The texts and the demo/code containers are slightly misaligned horizontally. Couldn't find where to play with this...

Fixed with b2dcfa0

  1. There are some cases where the demo and code container don't have border-radius applied. No idea why...

Fixed with 367273c

  1. Couldn't find where to modify the colors of the filled and outlined demo container on dark mode.

We talked offline, we need to find better color over here - https://github.com/mui-org/material-ui/blob/next/docs/src/modules/components/Demo.js#L110

  1. Couldn't find where to modify the max-width of the container that holds all the content but I played with some values directly on the inspector. This is something I personally think it makes for a much better reading experience (less horizontal movement of the eye) and also for a more overall balanced look since we gain negative space between the sidebar and the table of contents.

You should be able to change it over here - https://github.com/mui-org/material-ui/blob/next/docs/src/modules/components/AppLayoutDocs.js#L36 Not sure what is the best value to be honest.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 24, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 24, 2021
@mnajdova
Copy link
Member Author

@danilo-leal leaving a note that we need to find better color for this use-case as well:

@@ -110,7 +110,7 @@ export const getDesignTokens = (mode: 'light' | 'dark') =>
({
palette: {
primary: blue,
divider: mode === 'dark' ? blueDark[400] : grey[200],
divider: mode === 'dark' ? blueDark[700] : grey[200],
Copy link
Member

Choose a reason for hiding this comment

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

image

Suggested change
divider: mode === 'dark' ? blueDark[700] : grey[200],
divider: mode === 'dark' ? blueDark[500] : grey[200],

I think it should be lighter than 700. So far, 700, 800, 900 are background.

flexShrink: 0,
position: 'sticky',
height: 'calc(100vh - 70px)',
overflowY: 'auto',
padding: theme.spacing(2, 2, 2, 0),
padding: theme.spacing(2, 5, 2, 0),
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure about this? that will reduce the space of text.

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

Left a couple of comments, the rest looks good.

@mnajdova
Copy link
Member Author

Left a couple of comments, the rest looks good.

I will let @danilo-leal respond. The changes are mostly his :)

@mnajdova
Copy link
Member Author

I am going to merge this one to leverage some fixes we already have as part of the PR. Will leave the unaddressed comments by @siriwatknp open, so that we can decide and address them in the next PR.

@mbrookes
Copy link
Member

mbrookes commented Aug 24, 2021

Hmm... the font for <code> doesn't align with the body text. It's quite noticeable. We can pick it up in the next iteration.

Also not convinced about the <h1> color - but that can be fixed at any point.

@mbrookes mbrookes merged commit ad4a676 into mui:next Aug 24, 2021
@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Aug 25, 2021
danilo-leal referenced this pull request in mnajdova/material-ui Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants