-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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] Support rendering markdown outside of docs #37691
[docs-infra] Support rendering markdown outside of docs #37691
Conversation
packages/markdown/loader.js
Outdated
const pageFilename = this.context | ||
.replace(this.rootContext, '') | ||
// Use .. as the docs runs from the /docs folder | ||
const repositoryRoot = path.join(this.rootContext, '..'); | ||
const fileRelativeContext = path.relative(repositoryRoot, this.context) | ||
// win32 to posix | ||
.replace(/\\/g, '/'); |
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.
This is the fix
Netlify deploy previewhttps://deploy-preview-37691--material-ui.netlify.app/ Bundle size report |
54902da
to
cb28c9a
Compare
cb28c9a
to
5117922
Compare
@@ -157,7 +160,7 @@ module.exports = async function demoLoader() { | |||
// The import paths currently use a completely different format. | |||
// They should just use relative imports. | |||
let moduleID = `./${demoName.replace( | |||
`pages/${pageFilename.replace(/^\/src\/pages\//, '')}/`, | |||
`pages/${fileRelativeContext.replace(/^docs\/src\/pages\//, '')}/`, |
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.
Mostly stylistic, but this line feels a bit strange. Isn't
`pages/${fileRelativeContext.replace(/^docs\/src\/pages\//, '')}/`
the same as
`${fileRelativeContext.replace(/^docs\/src\//, '')}/`
or even
`${path.posix.relative('docs/src', fileRelativeContext)}/`
?
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.
Yeah, not sure why it was done this way.
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.
I noticed the button points to
https://github.com/mui/material-ui/blob/master/docs/data/material/getting-started/overview/overview.md
It could make sense to let it open the editor directly:
https://github.com/mui/material-ui/edit/master/docs/data/material/getting-started/overview/overview.md
@Janpot Ok, deal: #37695. I like how it saves one step. But also how it creates a more explicit user journey for those that are not familiar with GitHub. With this proposal, it shows a clear create a GitHub account page when you don't have one. Today, it's only a greyed edit button, with no clue what you are doing wrong. ![]() |
Fix the edit page button in https://mui.com/toolpad/examples/qr-generator/, reported in https://app.ahrefs.com/site-audit/3833384/52/data-explorer?columns=pageRating%2Curl%2Ctraffic%2ChttpCode%2CcontentType%2Cdepth%2CincomingAllLinks&filterId=9b88587038fc9bb2e08e0ff143d5f328&issueId=c64d89a6-d0f4-11e7-8ed1-001e67ed4656&sorting=-pageRating
The issue is that the markdown page wasn't inside the /docs folder: https://github.com/mui/mui-toolpad/blob/6937af716d3caa7dfbfa5b1eb737bea67d1ffcf4/docs/pages/toolpad/examples/qr-generator.js#L7.