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] Type interface API pages #14138

Merged
merged 14 commits into from
Sep 11, 2024

Conversation

alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Aug 8, 2024

Follow up of mui/material-ui#43128

This is the opportunity to find incosistency about description and fix them.

Available slots for description

  1. The meta data (the one displayed on google search)
  2. Just after the page H1 title
  3. Just before props definitions
Illustration

Available descriptions

a. A generic description with a placeholder for the component name:

API reference docs for the React {{componentName}} component. Learn about the props, CSS, and other APIs of this exported module."

b. The custom description from the JSDocs of the component (when provided)

Current behavior

On component API

  1. a (generic def)
  2. a (generic def)
  3. b (custome def)

On interface API

  1. a (generic def) with wrong content 🙈
  2. b (custom def)
  3. Nothing

Proposal

  • Fix the generic definition with the following sentence (replacing props and classes by "properties")

    API reference docs for the {{name}} interface. Learn about the properties other APIs of this exported module.

  • Follow the same patern as the API page of other components

@alexfauquette alexfauquette added the scope: docs-infra Specific to the docs-infra product label Aug 8, 2024
@mui-bot
Copy link

mui-bot commented Aug 8, 2024

Deploy preview: https://deploy-preview-14138--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 3cafec7

@JCQuintas
Copy link
Member

I don't know why types are not found for stuff imported from 'docs/src/...' but should not be an issue when moved to the @mui/docs package

Which types do you mean?

@alexfauquette
Copy link
Member Author

alexfauquette commented Aug 8, 2024

The one I import from @mui/monorepo/docs/ instead of docs/

But not an issue since those stuff will move to the docs package

Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 14, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 27, 2024
@alexfauquette alexfauquette marked this pull request as ready for review August 27, 2024 09:34
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 27, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 29, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 29, 2024
@alexfauquette alexfauquette requested a review from a team August 29, 2024 14:17
@@ -131,26 +186,17 @@ export default function ApiPage(props) {
language="jsx"
/>

{componentDescription ? (
Copy link
Member

Choose a reason for hiding this comment

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

Wondering what componentDescription was used for - is it okay to discard completely?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, component descript does not exist for interfaces. It has interfaceDescription which is already used a bit above in

<Typography
  variant="h5"
  component="p"
  className="description"
  gutterBottom
  dangerouslySetInnerHTML={{ __html: interfaceDescription }}
/>

It's visible for example in GridActionsColDef API

image

I replace the usual API page description

API reference docs for the React {{componentName}} component. Learn about the props, CSS, and other APIs of this exported module.

But that could probably be improved. I will check with other X memebers what they want for this page

@@ -142,5 +142,8 @@
"/components/backdrop": "Backdrop",
"/components/alert": "Alert",
"/components/pagination": "Pagination"
},
"api-docs": {
"interfacePageDescription": "API reference docs for the {{name}} interface. Learn about the properties other APIs of this exported module."
Copy link
Member

Choose a reason for hiding this comment

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

Maybe have the default be something like:

Suggested change
"interfacePageDescription": "API reference docs for the {{name}} interface. Learn about the properties other APIs of this exported module."
"interfacePageDescription": "Extended documentation for the {{name}} interface with detailed information on the module's properties and available APIs."

type: { description: string };
default?: string;
required?: true;
isProPlan?: true;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason not to have a plan?: 'pro' | 'premium' | 'community' here?

Copy link
Member Author

@alexfauquette alexfauquette Sep 6, 2024

Choose a reason for hiding this comment

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

No other reason than "that's what the script returns"

I tried to do the update, but that would require a PR on the core repo to update properties section component

https://github.com/mui/material-ui/blob/master/docs/src/modules/components/ApiPage/sections/PropertiesSection.tsx/#L66-L69

Copy link
Member

Choose a reason for hiding this comment

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

Would probably make sense to update it at some point, but it's out of the scope of this PR then 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: docs-infra Specific to the docs-infra product size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants