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] Automatically generate API docs #1529

Merged
merged 58 commits into from
Jun 3, 2021
Merged

Conversation

m4theushw
Copy link
Member

@m4theushw m4theushw commented Apr 30, 2021

This PR covers the automatic generation of the API pages using typedoc.

Preview:
https://deploy-preview-1529--material-ui-x.netlify.app/api/data-grid/

Closes #446
To be merged after #1572

Summary of changes

  1. All API pages of the DataGrid are not in a new submenu under Components API.

image

  1. Added a page to group the exports into categories. This is what users looking for the DataGrid component will see now.

  2. Added a page describing how to subscribe to events. This page also contains a table automatically populated with all the events. I had to do some workarounds to get the same styling from the component API page here. The worst part I think is using the MarkdownElement (a component from the docs) inside a demo, but users can't fork the demo so maybe we can live with that. We could separate the events into groups if necessary.

  3. Generated API pages for the GridApi, GridColDef, GridCellParams and GridRowParams. Which other APIs should we have documentation?


TODO:

@m4theushw m4theushw force-pushed the generate-docs branch 3 times, most recently from bbd65da to 762789f Compare May 6, 2021 01:55
@m4theushw m4theushw marked this pull request as ready for review May 6, 2021 02:24
@dtassone
Copy link
Member

dtassone commented May 6, 2021

Nice 👍

A couple of points.

  • The GridApi is the aggregation of all the hooks therefore, when we reach the proper level of pluginability, this one will be sort of dynamic, and maybe hooks will have to register their api somehow.
    Therefore, maybe we should consider splitting the page in api sections, such as
GridCoreApi
....
GridRowApi
...
  • The events page is a great addition, however I believe it misses a bit of context. My idea was to put this page in a parent Advanced page.
    this page would have the following section/sub pages depending on the length
Advanced
 - How the grid works? (the pattern...)
 - ApiRef (How to use it...)
 - Events (What you have done)
 - Components (how to create component, using useGridState...)

For the events page, I would add a warning or disclaimer to say something like Please note that you have to return the subscription in useEffect for it to be disposed correclty when the component unmount
Maybe a comment in the demo?

React.useEffect(() => {
// Please note that you have to return the subscription in useEffect for it to be disposed correclty when the component unmount
   return apiRef.current.subscribeEvent('columnResize', (params) => {...}, []
}

@m4theushw
Copy link
Member Author

The GridApi is the aggregation of all the hooks therefore, when we reach the proper level of pluginability, this one will be sort of dynamic, and maybe hooks will have to register their api somehow.

I would leave the GridApi page with all the methods it has. This interface is what users will get when they call apiRef.current.

Your idea of splitting the API into sections is nice, since we have a lot of methods. But I would rather not create a page for them but generate a JSON that, with a custom component, we can embed it into the markdown docs. On each feature page (sorting, filtering, pagination, selection) we could have a section like "Controlling this feature programmatically" with the methods available (https://material-ui.com/components/data-grid/sorting/#apiref).

The events page is a great addition, however I believe it misses a bit of context. My idea was to put this page in a parent Advanced page.

An "Advanced Use" page is good, but I would prefer to do it in another PR. For now, we could move the catalog of events to https://deploy-preview-1529--material-ui-x.netlify.app/api/data-grid/. What do you think?

@dtassone
Copy link
Member

dtassone commented May 7, 2021

For the apiPage I was talking about sections not separate pages.

An "Advanced Use" page is good, but I would prefer to do it in another PR. For now, we could move the catalog of events to https://deploy-preview-1529--material-ui-x.netlify.app/api/data-grid/. What do you think?

I think we can leave the events page as it is for now. Then open a new PR to add some more context or start the advanced page.

Issue on https://deploy-preview-1529--material-ui-x.netlify.app/api/x-grid/ => broken links around the doc

Also not sure about the menu.

image

Is it possible to show the api reference page as a landing page when you click the header?

@dtassone dtassone mentioned this pull request May 7, 2021
@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label May 8, 2021
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.

Look like we are a long way done. 👍 for the hybrid approach taken, half automation looks great

docs/pages/api-docs/data-grid/index.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
docs/pages/api-docs/data-grid/grid-api.md Outdated Show resolved Hide resolved
Comment on lines +33 to +35
## Catalog of events

{{"demo": "pages/components/data-grid/events/CatalogOfEvents.js", "bg": "inline", "hideToolbar": true}}
Copy link
Member

Choose a reason for hiding this comment

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

We would need to explain the upper-case import conversion logic.

) => () => void;
```

{{"demo": "pages/components/data-grid/events/SubscribeToEvents.js", "bg": "inline", "defaultCodeOpen": false}}
Copy link
Member

Choose a reason for hiding this comment

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

Was the plan to use GRID_COLUMN_RESIZE in the demos or columnResize? I can't remember.

Copy link
Member Author

Choose a reason for hiding this comment

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

We had this discussion in #1299 (comment). The plan was to use columnResize in demos and markdown. GRID_COLUMN_RESIZE could be documented, but users wouldn't be encouraged to use.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, cc @mui-org/x to make sure we all agree on the approach. I personally don't really mind.

The advantage of GRID_COLUMN_RESIZE is IMHO to be easier to search, but since apiRef.current.subscribe is easy to grep in a codebase, I don't see it really important.

docs/src/pages/components/data-grid/events/events.md Outdated Show resolved Hide resolved
docs/pages/api-docs/data-grid/grid-api.md Outdated Show resolved Hide resolved
docs/pages/api-docs/data-grid/grid-api.md Outdated Show resolved Hide resolved
import path from 'path';
import kebabCase from 'lodash/kebabCase';
import * as prettier from 'prettier';
import { renderInline as renderMarkdownInline } from '../../node_modules/@material-ui/monorepo/docs/src/modules/utils/parseMarkdown';
Copy link
Member

Choose a reason for hiding this comment

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

This could work

Suggested change
import { renderInline as renderMarkdownInline } from '../../node_modules/@material-ui/monorepo/docs/src/modules/utils/parseMarkdown';
import { renderInline as renderMarkdownInline } from '@material-ui/monorepo/docs/src/modules/utils/parseMarkdown';

Copy link
Member Author

@m4theushw m4theushw May 10, 2021

Choose a reason for hiding this comment

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

The renderInline is only available in the next branch, but the monorepo used by the docs points to the master branch. That's why I'm importing it from the node_modules of the root directory.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 8, 2021

Also, before we publish these pages live, it would be awesome to review in detail what we document, adding @ignore anything that looks suspicious in the docs. Basically, I think that we should default to no documentation, so we can asset it afterward if it's really valuable (the stronger the constraints, the impose, the more room for implementing features we have).

@m4theushw
Copy link
Member Author

What is left in this one? Can we merge it and improve it at a later stage?
As always it won't be perfect at the first shot, but it needs to be in so we can improve it

@dtassone What was missing is the descriptions of events and methods, but that is already fixed and merged (#1572 and #1767). I think we can push forward and #1529 (comment) can be done in another PR.

docs/package.json Outdated Show resolved Hide resolved
Comment on lines -1 to -2
/* eslint-disable */
import * as babel from '@babel/core';
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure we will add this one back in the future for DataGrid, XGrid, GridToolbarExport, TreeView, etc. We are not here yet.

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.

[docs] Generate GridColDef api page
3 participants