-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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] Move data grid interfaces to standard API page layout #12016
Conversation
Deploy preview: https://deploy-preview-12016--material-ui-x.netlify.app/ |
db0b5ce
to
dee3bdc
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
5fd3baa
to
227b903
Compare
✅ Deploy Preview for material-ui-x ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
let planImg: string; | ||
if (property.projects.includes('x-data-grid')) { | ||
planImg = ''; | ||
} else if (property.projects.includes('x-data-grid-pro')) { | ||
planImg = | ||
' [<span class="plan-pro" title="Pro plan"></span>](/x/introduction/licensing/#pro-plan)'; | ||
} else if (property.projects.includes('x-data-grid-premium')) { | ||
planImg = | ||
' [<span class="plan-premium" title="Premium plan"></span>](/x/introduction/licensing/#premium-plan)'; | ||
} else { | ||
throw new Error(`No valid plan found for ${property.name} property in ${object.name}`); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extracted in getPlanLevel()
const formattedName = property.isOptional | ||
? `<span class="prop-name optional">${property.name}<sup><abbr title="optional">?</abbr></sup>${planImg}</span>` | ||
: `<span class="prop-name">${property.name}${planImg}</span>`; | ||
|
||
const formattedType = `<span class="prop-type">${escapeCell(property.typeStr)}</span>`; | ||
|
||
const formattedDefaultValue = | ||
defaultValue == null ? '' : `<span class="prop-default">${escapeCell(defaultValue)}</span>`; | ||
|
||
const formattedDescription = escapeCell( | ||
linkify(property.description, documentedInterfaces, 'markdown'), | ||
); | ||
|
||
if (hasDefaultValue) { | ||
text += `| ${formattedName} | ${formattedType} | ${formattedDefaultValue} | ${formattedDescription} |\n`; | ||
} else { | ||
text += `| ${formattedName} | ${formattedType} | ${formattedDescription} |\n`; | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting and linkify is done when generating the JSON. The rendering aspect (column order, adding plan image, ...) is done in the PropsTable
component done in mui/material-ui#41069
} | ||
const demos = tagInfo.text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it was markdown rendering, the interface description used markdown format to add links to demo
Here I parse them to extract the url and the name to render
.replace(/>/g, '>') | ||
.replace(/\|/g, '\\|') | ||
.replace(/\r?\n/g, '<br />'); | ||
return value.replace(/</g, '<').replace(/>/g, '>').replace(/\r?\n/g, '<br />'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this escape aspect because it's only for markdown
.replace(/\|/g, '\\|')
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
47e5f89
to
d65cfeb
Compare
d65cfeb
to
6d13431
Compare
description={description} | ||
disableToc={false} | ||
toc={[]} | ||
location={apiSourceLocation} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Edit this page" button is broken – it links to https://github.com/mui/mui-x/settings/master
Previously, we were linking to the .md
file, but did it make much sense if the file was autogenerated?
I think we should remove the "Edit this page" button from the interfaces pages.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link point to https://github.com/mui/mui-x/edit/master/[the location]
but the location is an empty string. And https://github.com/mui/mui-x/edit/master/
goes to settings because it's a folder, not a file
Will be fixed with this PR.
@@ -31,14 +31,18 @@ interface ParsedProperty { | |||
name: string; | |||
description: string; | |||
tags: { [tagName: string]: ts.JSDocTagInfo }; | |||
isOptional: boolean; | |||
required: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PropertiesSection
expects the required
field.
required: property.required, | ||
})) | ||
.sort((a, b) => { | ||
if ((a.required && b.required) || (!a.required && !b.required)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To list the required properties first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! 👏🏻
These pages look so much better now, especially this one:
Before: https://deploy-preview-12015--material-ui-x.netlify.app/x/api/data-grid/grid-aggregation-function/
After: https://deploy-preview-12016--material-ui-x.netlify.app/x/api/data-grid/grid-aggregation-function/
…into data-grid-interface
Co-authored-by: Andrew Cherniavskyi <andrew@mui.com>
Co-authored-by: Andrew Cherniavskyi <andrew@mui.com>
Move interface API pages to the same behavior as other pages:
Moving this script to the core package seems tricky, because core has a behavior where each project can be handled independently. Whereas we have interfaces shared between multiple projects and we need to access to them together to do know if properties are available in community, pro; or premium
Still todo:
Examples
GridAPI
ColDef