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

[DataGrid] Remove unnecessary syntax in JSDoc #10567

Merged
merged 4 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/pages/x/api/data-grid/data-grid-premium.json
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@
"name": "union",
"description": "{ clipboardExport?: bool, csvExport?: bool }<br>&#124;&nbsp;bool"
},
"default": ": false"
"default": "false"
},
"unstable_onCellSelectionModelChange": {
"type": { "name": "func" },
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/x/api/data-grid/data-grid-pro.json
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@
"name": "union",
"description": "{ clipboardExport?: bool, csvExport?: bool }<br>&#124;&nbsp;bool"
},
"default": ": false"
"default": "false"
}
},
"slots": [
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/x/api/data-grid/data-grid.json
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@
"name": "union",
"description": "{ clipboardExport?: bool, csvExport?: bool }<br>&#124;&nbsp;bool"
},
"default": ": false"
"default": "false"
}
},
"slots": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ export interface AddPathToDemoDataOptions {

/**
* The depth of the tree
* @default: 1
* @default 1
*/
maxDepth?: number;

Lev-Shapiro marked this conversation as resolved.
Show resolved Hide resolved
/**
* The average amount of children in a node
* @default: 2
* @default 2
*/
averageChildren?: number;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,7 @@ DataGridPremiumRaw.propTypes = {
/**
* If `true`, the grid will not use `valueFormatter` when exporting to CSV or copying to clipboard.
* If an object is provided, you can choose to ignore the `valueFormatter` for CSV export or clipboard export.
* @default: false
* @default false
Copy link
Member

Choose a reason for hiding this comment

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

This is technically touching non-MIT code.
Do we need to go with the SLA process for even non-functional changes?
cc @joserodolfofreitas @DanailH

Copy link
Member

Choose a reason for hiding this comment

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

@LukasTy I believe we need a CLA for any change in the commercially licensed folder.

Copy link
Member

Choose a reason for hiding this comment

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

Although in our CLA docs it says:

If the change is nontrivial and user-facing, the CLA needs to be signed.

Copy link
Member

@oliviertassinari oliviertassinari Oct 10, 2023

Choose a reason for hiding this comment

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

I have updated https://www.notion.so/mui-org/CLA-Contributor-License-Agreement-92ece655b1584b10b00e4de9e67eedb0?pvs=4#ede845886e494cb4b802a67563152130 to detail my take on this topic, inspired by OpenSSL.

Here, I think we can consider it's a trivial change, if the author agrees too, we could move forward without signing a CLA.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sure, let's move forward

*/
unstable_ignoreValueFormatterDuringExport: PropTypes.oneOfType([
PropTypes.shape({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,7 @@ DataGridProRaw.propTypes = {
/**
* If `true`, the grid will not use `valueFormatter` when exporting to CSV or copying to clipboard.
* If an object is provided, you can choose to ignore the `valueFormatter` for CSV export or clipboard export.
* @default: false
* @default false
*/
unstable_ignoreValueFormatterDuringExport: PropTypes.oneOfType([
PropTypes.shape({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export interface GridGroupingColDefOverride<R extends GridValidRowModel = any>
* The field from which we want to apply the sorting and the filtering for the grouping column.
* It is only useful when `props.rowGroupingColumnMode === "multiple"` to decide which grouping criteria should be used for sorting and filtering.
* Do not have any effect when building the tree with the `props.treeData` feature.
* @default: The sorting and filtering is applied based on the leaf field in any, otherwise based on top level grouping criteria.
* @default The sorting and filtering is applied based on the leaf field in any, otherwise based on top level grouping criteria.
Copy link
Member

Choose a reason for hiding this comment

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

this is not the intended usage of the @default directive. @cherniavskii do we use it that way in the codebase very often?

Copy link
Contributor Author

@Lev-Shapiro Lev-Shapiro Oct 4, 2023

Choose a reason for hiding this comment

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

I guess I could also answer this question. I've found yesterday many incorrect usages of the @default directive some have syntax problems and some incorrect usages of that directive, I could try make a more global PR and an issue to fix it, sounds good?

Copy link
Member

Choose a reason for hiding this comment

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

It's used in a similar way in the Pickers codebase a few times as well. 🤔
Maybe it's not the most correct usage, but it works and helps us to build the API documentation that would be useful. 🙈

Copy link
Member

Choose a reason for hiding this comment

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

@Lev-Shapiro thanks, but that's not what I intended with that comment. I know it is not the correct usage of it, but my intention was to know if we use it that way elsewhere or if it is the only time.

As mentioned by @LukasTy it does not pose any threat and if it is a valuable tool we should continue using it like this! 💯

*/
mainGroupingCriteria?: string;

Expand All @@ -27,7 +27,7 @@ export interface GridGroupingColDefOverride<R extends GridValidRowModel = any>

/**
* If `true`, the grouping cells will not render the amount of descendants.
* @default: false
* @default false
*/
hideDescendantCount?: boolean;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/grid/x-data-grid/src/DataGrid/DataGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ DataGridRaw.propTypes = {
/**
* If `true`, the grid will not use `valueFormatter` when exporting to CSV or copying to clipboard.
* If an object is provided, you can choose to ignore the `valueFormatter` for CSV export or clipboard export.
* @default: false
* @default false
*/
unstable_ignoreValueFormatterDuringExport: PropTypes.oneOfType([
PropTypes.shape({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ export interface DataGridPropsWithDefaultValues {
/**
* If `true`, the grid will not use `valueFormatter` when exporting to CSV or copying to clipboard.
* If an object is provided, you can choose to ignore the `valueFormatter` for CSV export or clipboard export.
* @default: false
* @default false
*/
unstable_ignoreValueFormatterDuringExport:
| boolean
Expand Down
Loading