Skip to content

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Aug 30, 2024

This is a quick follow-up on #43387.

This link creates a 301, which slows the user experience by about 200ms:

SCR-20240830-oiqr

I keep forgetting about tree (folder) vs. blob (file) in the URL to link in GitHub.


Also, none of the other chips have target blank, I got confused that it didn't open in the same window. I removed it for consistency of behavior. We could discuss adding to all the chips as well, but I think it's a bit beyond this scope.

Preview: https://deploy-preview-43534--material-ui.netlify.app/material-ui/react-alert/

@oliviertassinari oliviertassinari added type: bug It doesn't behave as expected. scope: docs-infra Changes related to the docs-infra product. labels Aug 30, 2024
@mui-bot
Copy link

mui-bot commented Aug 30, 2024

Netlify deploy preview

https://deploy-preview-43534--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 35c3220

@@ -107,14 +107,13 @@ export function ComponentLinkHeader(props: ComponentLinkHeaderProps) {
size="small"
variant="outlined"
rel="nofollow"
href={`${process.env.SOURCE_CODE_REPO}/blob/v${process.env.LIB_VERSION}/${headers.githubSource}`}
href={`${process.env.SOURCE_CODE_REPO}/tree/v${process.env.LIB_VERSION}/${headers.githubSource}`}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
href={`${process.env.SOURCE_CODE_REPO}/tree/v${process.env.LIB_VERSION}/${headers.githubSource}`}
href={`${process.env.SOURCE_CODE_REPO}/${/\.[a-z]{2,3}$/.test(headers.githubSource) ? 'blob' : 'tree'}/v${process.env.LIB_VERSION}/${headers.githubSource}`}

Maybe I'm overengineering But with a regexp that check if the file ends with a dot plus 2 or 3 characters, you get a fix that work for all cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would imagine that on the API page we always link files and on the demo page folders, so it might be fine?
If we start to see regressions, yeah, I would add an exception here, it must be a folder.

Copy link
Member

Choose a reason for hiding this comment

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

This component is not on the API page. It's the header of the demo. Technically, we could put files or folders in the markdown header

@oliviertassinari oliviertassinari merged commit abbfbd5 into mui:master Aug 31, 2024
21 checks passed
@oliviertassinari oliviertassinari deleted the docs-infra-fix-301 branch August 31, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: docs-infra Changes related to the docs-infra product. type: bug It doesn't behave as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants