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

[code-infra] Simplify bug reproduction #11849

Merged
merged 8 commits into from
Feb 4, 2024

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Jan 28, 2024

It's not directly linked to the milestone of moving from CodeSandbox to StackBlitz but still is directly connected. It's complex to have to manage CodeSandbox templates, I believe we can simplify the contribution experience by heavily relying on demo forks. The combinator to support with CodeSandbox templates is high (Material UI / Base UI / Joy UI, Pro / MIT, Date Picker / Data Grid, TypeScript/JavaScript)

Preview: https://deploy-preview-11849--material-ui-x.netlify.app/x/introduction/support/#bug-reproductions (image fixed from mui/material-ui#40833)

@oliviertassinari oliviertassinari added the scope: code-infra Specific to the core-infra product label Jan 28, 2024
@mui-bot
Copy link

mui-bot commented Jan 28, 2024

Deploy preview: https://deploy-preview-11849--material-ui-x.netlify.app/

Updated pages:

Generated by 🚫 dangerJS against f3dba0e

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 28, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

If you don't have one, you can use one of these options:
- [DataGrid codesandbox template](https://codesandbox.io/s/github/mui/mui-x/tree/master/templates/x-data-grid?file=/src/demo.tsx)
- Fork any of the examples in our [documentation](https://mui.com/x/introduction/) by [clicking on the codesandbox or stackblitz icon](https://mui.com/static/docs/forking-an-example.png)
Please provide a link to a live example and an unambiguous set of steps to reproduce this bug. See our [documentation](https://mui.com/x/introduction/support/#bug-reproductions) on how to build a reproduction case.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think having the links directly in the issue template is faster, there is no need to click & load another page. It's also nice to have the code template because it doesn't require to search through the docs, users can just insert their code directly.

Copy link
Member Author

@oliviertassinari oliviertassinari Jan 28, 2024

Choose a reason for hiding this comment

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

users can just insert their code directly.

This sandbox to be most helpful needs the user:

  • to use the Pro plan
  • to use the Data Grid
  • to use TypeScript
  • to use Material UI

I'm not sure it's so frequent. The docs support a lot of combinatory.

Copy link
Contributor

@romgrk romgrk Jan 29, 2024

Choose a reason for hiding this comment

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

The packages for community & premium are installed in the template, we could add commented lines like:

// Uncomment the line that applies
// import { DataGrid as DataGrid } from 'x-data-grid'
// import { DataGridPro as DataGrid } from 'x-data-grid-pro'
// import { DataGridPremium as DataGrid } from 'x-data-grid-premium'

And MUI & typescript are the most common choices. If I was a user, I feel like I would be more satisfied to have the template available right there rather than to have to go through multiple pages/links. I think the other packages (charts, pickers, tree-view) should also include a link like this one in this section. It also already works as-is with stackblitz.

Copy link
Member Author

@oliviertassinari oliviertassinari Jan 31, 2024

Choose a reason for hiding this comment

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

I have tried an alternative tradeoff, keeping this bug reproduction template.

// Uncomment the line that applies

I don't believe developers follow this flow. I don't recall the last time I wrote data grid code from scratch, I always copied it from my own codebase or from the docs. I don't know the API well enough, maybe it's possible for developers who use this all day. My guess is that developers if they report a bug, they likely hit an edge case that isn't straight to reproduce. I mean, if it's straight to reproduce, then forking a demo of the docs sounds fast, if it's complex, then likely you would copy and paste your codebase.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe developers follow this flow. I don't recall the last time I write data grid code from scratch, I always copied it from my own codebase or from the docs.

Very true.

I still think less clicks is a better UX, but I don't open that many issues with the templates so I'm fine with this change as it is.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 28, 2024
Copy link
Member

@michelengelen michelengelen left a comment

Choose a reason for hiding this comment

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

Just a few nits in the wording... feel free to ignore.

docs/data/introduction/support/support.md Outdated Show resolved Hide resolved
docs/data/introduction/support/support.md Outdated Show resolved Hide resolved
Co-authored-by: Michel Engelen <[email protected]>
Signed-off-by: Olivier Tassinari <[email protected]>
Signed-off-by: Olivier Tassinari <[email protected]>
Signed-off-by: Olivier Tassinari <[email protected]>
@oliviertassinari oliviertassinari enabled auto-merge (squash) February 4, 2024 16:53
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Feb 4, 2024

The image is broken, but will work once a new Core release is made. I will cherry pick it. This gets cherry picked in MUI X v6 as well for the GitHub issue template link to work.

@oliviertassinari oliviertassinari merged commit 819539f into mui:next Feb 4, 2024
15 checks passed
@oliviertassinari oliviertassinari deleted the simplify-reproduction branch February 4, 2024 21:24
oliviertassinari added a commit that referenced this pull request Feb 4, 2024
Signed-off-by: Olivier Tassinari <[email protected]>
Co-authored-by: Michel Engelen <[email protected]>
@oliviertassinari
Copy link
Member Author

alexfauquette pushed a commit to alexfauquette/mui-x that referenced this pull request Feb 5, 2024
Signed-off-by: Olivier Tassinari <[email protected]>
Co-authored-by: Michel Engelen <[email protected]>
@alexfauquette
Copy link
Member

It's live: mui.com/x/introduction/support/#bug-reproductions

I've open cherry-pick request to avoid overriding it during the git push force of the docs deployement

#11932

oliviertassinari added a commit that referenced this pull request Feb 5, 2024
Signed-off-by: Olivier Tassinari <[email protected]>
Co-authored-by: Olivier Tassinari <[email protected]>
Co-authored-by: Michel Engelen <[email protected]>
thomasmoon pushed a commit to thomasmoon/mui-x that referenced this pull request Sep 9, 2024
Signed-off-by: Olivier Tassinari <[email protected]>
Co-authored-by: Michel Engelen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: code-infra Specific to the core-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants