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 API page deprecation info #40440

Merged
merged 8 commits into from
Jan 11, 2024

Conversation

DiegoAndai
Copy link
Member

@DiegoAndai DiegoAndai commented Jan 5, 2024

Summary

  • Standardize API page warnings
  • Add "DEPRECATED" label to props and class lists
  • Add support for class deprecation

Details of each are below

Standardize API page alert

Introduce the ApiWarning component to standardize all API page warning alerts: deprecations and "needs to be able to hold a ref" alerts on both table and list views.

This doesn't include other warnings throughout the docs, which might be something we should look into but goes beyond the scope of this PR.

Screenshot 2024-01-09 at 19 47 12 Screenshot 2024-01-09 at 19 46 37

Add "DEPRECATED" label to props and class lists

This indicates deprecations when lists are collapsed. Without this change, deprecations cannot be seen until the prop is expanded.

Screenshot 2024-01-09 at 16 24 32

Add support for class deprecation in API docs:

This is required, for example, for #40418. Here's an example of how it would look like.

Screenshot 2024-01-05 at 12 27 23

Notes

This is my first docs-infra PR, so please correct anything I might not be aware of 😊.

@DiegoAndai DiegoAndai added the scope: docs-infra Specific to the docs-infra product label Jan 5, 2024
@DiegoAndai DiegoAndai requested a review from a team January 5, 2024 16:00
@DiegoAndai DiegoAndai self-assigned this Jan 5, 2024
@mui-bot
Copy link

mui-bot commented Jan 5, 2024

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against ebde672

@alexfauquette
Copy link
Member

What about splitting the information "there is a deprecation" with isDeprecated in the json you're modifying, and saving the description of the deprecation in the translationFolder. The goal being to keep all stuff to translate in a single file

@michaldudak Could you confirm it's what you intended to do in your refacto of this script?

@siriwatknp
Copy link
Member

image

Is it possible to add DEPRECATED label similar to STATE? This would make it easy to see when the content is not collapsed.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 8, 2024
@michaldudak
Copy link
Member

We need translated text in the translations JSON for sure.
Looking at this now, I think I unnecessarily added the description field to the classes array items (it should live just in translations, similarly to props), causing duplication.
Let's follow how the props work and have the translatable text in the translations directory, not in the main API definition JSON.

@DiegoAndai DiegoAndai force-pushed the docs-infra-class-deprecations branch from 40af114 to d065376 Compare January 9, 2024 19:13
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 9, 2024
@DiegoAndai DiegoAndai changed the title [docs-infra] Add support for class deprecations in api docs [docs-infra] Improve API page deprecation info Jan 9, 2024
@michaldudak
Copy link
Member

The implementation looks good.
One issue I found is when a state class is deprecated, the "deprecated" tag isn't shown on the list view, but the "state" tag is colored differently:
image

@DiegoAndai
Copy link
Member Author

@alexfauquette @danilo-leal, this is ready for your re-review 😊 I updated the description with the added changes.

I didn't know how to figure out why the MUI X deprecation warnings in their API pages didn't update. I thought these were shared.

@alexfauquette
Copy link
Member

alexfauquette commented Jan 11, 2024

I didn't know how to figure out why the MUI X deprecation warnings in their API pages didn't update. I thought these were shared.

Effectively the X docs is depending on the core one. So after merging this PR X will be able to update it's dependency to the monorepo and the get the update 👍

I made a try using your branch, but seems you don't have deprecated classes, nothing change

Copy link
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

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

Sound very nice. Especially having a single component for warnings 🙏

Could you update the #40418 to be able to see the components with their last version of the modification?

docs/src/modules/components/ApiPage/list/ClassesList.tsx Outdated Show resolved Hide resolved
@DiegoAndai
Copy link
Member Author

Could you update the #40418 to be able to see the components with their last version of the modification?

Done 😊: https://deploy-preview-40418--material-ui.netlify.app/material-ui/api/accordion-summary/#classes

@alexfauquette

Copy link
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

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

Amazing 🎉

Copy link
Contributor

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

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

Looks awesome! 🤙

@DiegoAndai DiegoAndai merged commit 60af051 into mui:master Jan 11, 2024
22 checks passed
@DiegoAndai DiegoAndai deleted the docs-infra-class-deprecations branch January 11, 2024 17:51
@oliviertassinari oliviertassinari added the new feature New feature or request label Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants