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 API for theme default prop #36490

Merged
merged 4 commits into from
Mar 22, 2023

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Mar 10, 2023

I initially only wanted to remove logic related to a given open-source project from the ApiPage so we can keep it more generic, e.g.

{(componentStyles.name || isJoyComponent) && (

there shouldn't be project specific logic like this, it's way too specific when it can be inside the json.

But then, I went a bit above to differentiate default prop customization to style customization. These are implemented differently now, especially with the data grid.

Before:

After:

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation scope: docs-infra Specific to the docs-infra product labels Mar 10, 2023
@mui-bot
Copy link

mui-bot commented Mar 10, 2023

Netlify deploy preview

@material-ui/core: parsed: +Infinity% , gzip: +Infinity%
@material-ui/lab: parsed: +Infinity% , gzip: +Infinity%
@material-ui/styles: parsed: +Infinity% , gzip: +Infinity%
@material-ui/private-theming: parsed: +Infinity% , gzip: +Infinity%
@material-ui/system: parsed: +Infinity% , gzip: +Infinity%
@material-ui/unstyled: parsed: +Infinity% , gzip: +Infinity%
@material-ui/utils: parsed: +Infinity% , gzip: +Infinity%
@mui/material-next: parsed: +Infinity% , gzip: +Infinity%
@mui/joy: parsed: +Infinity% , gzip: +Infinity%

Bundle size report

Details of bundle changes

Generated by 🚫 dangerJS against 0b531a4

@oliviertassinari oliviertassinari force-pushed the fix-component-name branch 2 times, most recently from 629e5e0 to 981cccd Compare March 10, 2023 18:24
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Mar 12, 2023

Follow-up thoughts:

  1. Shouldn't root be at the top: https://deploy-preview-36490--material-ui.netlify.app/joy-ui/api/button/#slots? [docs] Keep slot code order in API docs #36499
  2. JoyButton is a bit strange to customize the default when the class name is MuiButton. IMHO, the correct way to handle this here is to isolate the emotion context between Joy UI and Material UI & use MuiButton.
  3. We miss the docs for the component prop in https://deploy-preview-36490--material-ui.netlify.app/joy-ui/api/alert/#props.
  4. In the card https://deploy-preview-36490--material-ui.netlify.app/joy-ui/api/card/, we have no docs about the root element class name. Should we?
  5. The className for the input slot is wrong in https://deploy-preview-36499--material-ui.netlify.app/base/api/slider-unstyled/#slots, it doesn't exist, and probably shouldn't
  6. Related to 4. we have no docs about the custom class names in https://deploy-preview-36499--material-ui.netlify.app/base/api/slider-unstyled/ like we have in https://deploy-preview-36499--material-ui.netlify.app/material-ui/api/slider/#css, shouldn't we?

@hbjORbj
Copy link
Member

hbjORbj commented Mar 15, 2023

(1) I opened a PR (#36519) for this today (didn't know you opened a PR for it already!). I think we can go with your PR.

(3) I will handle this.

(4) I think we should. I can take of this. #36540

(5) I will handle this. #36539

(6) cc @michaldudak @siriwatknp

@hbjORbj
Copy link
Member

hbjORbj commented Mar 16, 2023

@oliviertassinari
(3) I noticed that component, slots, slotProps are missing in API docs of some components of all Base, Material and Joy. Were we aware of this?

For example, PopperUnstyled is missing component, and Material UI Avatar is missing slots and slotProps.

EDIT: Base components will no longer have component prop (Marija will work on this, I think).

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 17, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 20, 2023
@siriwatknp
Copy link
Member

siriwatknp commented Mar 20, 2023

For (6), I think we can extract the description and put it above the table (in a separate PR). This will also fix #36328 (comment).

image

@oliviertassinari oliviertassinari merged commit 16efe9d into mui:master Mar 22, 2023
@oliviertassinari oliviertassinari deleted the fix-component-name branch March 22, 2023 20:42
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 scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants