search

mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.16k stars 1.3k forks source link

[pickers] Missing Demo & API links JSDoc on certain components #9226

Closed nathantew14 closed 8 months ago

nathantew14 commented 1 year ago

Duplicates

Related page

https://mui.com/x/api/date-pickers/mobile-date-picker

Kind of issue

Missing information

Issue description

With "@mui/x-date-pickers": "^5.0.20":

Screenshot 2023-05-31 at 11 01 31 PM

With "@mui/x-date-pickers": "^6.5.0":

Screenshot 2023-05-31 at 10 58 22 PM

Starball on Stackoverflow answered my question with a detailed look at when the docstrings disappeared from the source code.

Context 🔦

It seems the docstrings have existed before, but have been seemingly removed intentionally. I'd like to know why, and maybe if they can be replaced.

LukasTy commented 1 year ago

Thank you for creating the issue and pointing out this inconsistency/regression! 💯 👍 It must have been forgotten during our v6 migration, where we temporarily had duplicate pickers components for the new and old variants and it got lost during the process. 🙈

alexfauquette commented 1 year ago

After some investigation, it seems this is a feature from core we don't support on X

image

Those decorations are auto-generated when running yarn docs:api thanks to this AST analyser

But it relies on the fact that elements are exported with export default and probably other stuff. Whereas in X we use export.

LukasTy commented 1 year ago

We do not seem to use the script linked above. MUI X uses https://github.com/mui/mui-x/blob/master/docs/scripts/api/buildComponentsDocumentation.ts to generate API documentation and there is no extra layer augmenting the JSDoc of component files. For @mui/xgrid it would seem that they could just manually add this JSDoc snippet as they technically have only 3 relevant top-level components. 🤔 As for the pickers... Well, it depends on how accurate we want the demo data to be, but it could be quite bothersome to generate a complete list of demos for every component. 🤔

LukasTy commented 1 year ago

We decided on going with a simple solution if possible. Add the JSDoc comments manually and ensure that scripts do not remove it. It could be sufficient to add JSDoc to the main components and not all the usable children components.

oliviertassinari commented 1 year ago

It looks like we miss these annotations on a lot of components:

michelengelen commented 1 year ago

Ongoing list of fixed reference links

DataGrid

Pickers

DatePicker

Time Pickers

Date Time Pickers

Date Range Pickers

Fields

Calendars

Clocks

Toolbars

Others

Charts

Area

Bar

Lines

Pie

Scatter

Shared components

previously done already!

michelengelen commented 1 year ago

As we are looking to consolidate the approach with core the current PRs will get closed for now.

Reasoning: (link for reference to the discussion https://github.com/mui/mui-x/pull/10626#discussion_r1354870105) The core product does add these JSDoc blocks based on the docs API. This makes a lot of sense since it reduces friction and is handling all missing blocks at once. This should also be future proof, so if there are no objections from @mui/x I will close the PRs and give a implementation of the core approach a try instead.

LukasTy commented 1 year ago

if there are no objections from mui/x I will close the PRs and give a implementation of the core approach a try instead.

Based on the information, setup and discussions in grooming we decided to go with a simple approach of manually adding the JSDoc comments. If you manage to find an automated solution relatively quickly, then sure, we can go with that, but we decided not to spend too much time on it, due to different setup and codebase. 🤔

michelengelen commented 1 year ago

Based on the information, setup and discussions in grooming we decided to go with a simple approach of manually adding the JSDoc comments.

That's an info I did not have yet ... I did briefly look at the problem yesterday and it is indeed a fair bit different, so it probably makes the most sense to just add this info manually.

Sry for the confusion ... I'll reopen the closed PRs 🙇

michelengelen commented 8 months ago

just noticed that all PRs are merged and we did miss to close the issue. Gonna do that now! 💪🏼

github-actions[bot] commented 8 months ago

:warning: This issue has been closed. If you have a similar problem, please open a new issue and provide details about your specific problem. If you can provide additional information related to this topic that could help future readers, please feel free to leave a comment.

How did we do @nathantew14? Your experience with our support team matters to us. If you have a moment, please share your thoughts through our brief survey.