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] Improve system properties page #23961

Merged
merged 6 commits into from Dec 12, 2020

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova commented Dec 11, 2020

This PR removes the group column from the properties tables, moving the links to the next column (System style function), where the links where possible are linking to the exact style function from that group.

In addition to this, the legend on the page is improved, by adding the Legend section and subsection for each of the columns.

The only other thing we may want to do is to move the Legend section after the table. Let me know what you think about this.

Link for review of the page - https://deploy-preview-23961--material-ui.netlify.app/system/properties/

@mui-pr-bot
Copy link

mui-pr-bot commented Dec 11, 2020

Details of bundle changes

Generated by 🚫 dangerJS against 0d191c2

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.

A couple of thoughts:

  • I'm slightly leaning toward legend after, but happy to follow @mbrookes direction on this
  • It would be interesting to add a reminder, just before the table that it only covers the custom properties, all the CSS properties are supported too.

@eps1lon eps1lon removed their request for review December 11, 2020 15:00
@@ -1,19 +1,30 @@
# Properties

<p class="description">This page lists all of the custom system properties, how are they linked with the theme, and which CSS properties they compute.</p>
<p class="description">This page lists all of the custom system properties, how are they linked with the theme, and which CSS properties they compute. All other regular CSS properties and selectors are supported too.</p>
Copy link
Member

Choose a reason for hiding this comment

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

Forgive me if I'm missing something obvious (not a first!), but in what sense are all are CSS selectors supported?

Copy link
Member Author

Choose a reason for hiding this comment

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

You can specify any valid CSS as javascript object, which includes use of pseudo selectors, like :hover, or class selectors, like & .MuiButton-root: {} etc

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I was thinking that "all" might be a bit broad, but I guess trying to caveat it would make it overly complicated.

@mbrookes
Copy link
Member

I'm slightly leaning toward legend after

Since the table is quite long, I think it's better to present the explanation first (with the sample as a reference), rather than hoping that the reader makes it to the bottom for an explanation.

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.

I would duplicate the information as it can be easily skipped.

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
docs/src/pages/system/properties/properties.md Outdated Show resolved Hide resolved
@mnajdova mnajdova merged commit 20eee93 into mui:next Dec 12, 2020
@zannager zannager added docs Improvements or additions to the documentation package: system Specific to @mui/system labels Jan 27, 2023
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 package: system Specific to @mui/system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants