[docs] Add links for demo in different deploys #26065
Conversation
eps1lon
force-pushed
the
docs/branch-switch
branch
from
91bf304
to
035dc4b
Compare
| @@ -363,6 +364,71 @@ export default function DemoToolbar(props) { | |||
| isFocusableControl, | |||
| }); | |||
|
|
|||
| const devMenuItems = []; | |||
| if (process.env.STAGING === true) { | |||
There was a problem hiding this comment.
I'm curious. Why not doing
| if (process.env.STAGING === true) { | |
| if (process.env.PULL_REQUEST === true) { |
As we already do in https://github.com/mui-org/material-ui/blob/33850e52ca32787fbc02b64d3225bac2d451612c/docs/src/modules/utils/getDemoConfig.js#L7
There was a problem hiding this comment.
I'm curious. Why not doing
Because I would use these links on all material-ui.netlify.app deploys
There was a problem hiding this comment.
Would it make sense to do the same with the muiCommitRef? I assume that it could work on all deploys as well
There was a problem hiding this comment.
Could you open a new issue explaining why it would make sense there? I don't understand why this is an issue besides "let's DRY out code at all cost".
There was a problem hiding this comment.
I don't think that it's about being DRY. I have asked because not being DRY could suggest that there might be an opportunity to solve a similar problem. It's not really important.
From what I understand, the extra value of using process.env.STAGING === true is to be able to get the menu on https://next--material-ui-x.netlify.app/ and then to compare with master. The value I see in using process.env.STAGING === true for the codesandbox link is to be able to test the behavior on HEAD, before it's released.
There was a problem hiding this comment.
Yeah, please open a new issue. I don't understand how a link using a codesandbox deploy would look outside of PRs. If it's not about DRY then I understand even less why this is raised here.
There was a problem hiding this comment.
I don't see codesandbox-ci build for the baseline branch, It might not work 😁. For the issue, I don't have the incentive to open one (I didn't see a strong pain it could solve). I think that we can snooze this topic.
| // eslint-disable-next-line react-hooks/rules-of-hooks -- process.env.STAGING never changes | ||
| const router = useRouter(); | ||
|
|
||
| const defaultReviewID = process.env.GIT_REVIEW_ID ?? '20000'; |
There was a problem hiding this comment.
Should we exit the whole branch instead of defaulting to 20000 if GIT_REVIEW_ID is missing?
There was a problem hiding this comment.
It's a template to get a link to another PR. 20000 is just an arbitrary number to make the PR number apparent in the created link.
|
/cc @mui-org/maintainers That might be a feature you're interested in. |
demo-links.mp4
Preview: http://deploy-preview-26065--material-ui.netlify.app/components/box
I often find myself switching between a PR and its base branch when doing manual testing. It was a bit annoying to constantly have to adjust these links manually so I just automatically include them now in deploys on
mui-org/material(or local development). material-ui.com deploys should be minimally affected due to dead-code-elimination.Also includes a permalink which was was not that trivial to get (always scrolled through netlify.com).