-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[DataGrid] Improve edit cell UI #1168
Conversation
I have added a new section in the storybook for dedicated visual regressions tests. I think that we can remove the visual regressions on the other stories. If these other stories are for testing edge cases, or stress tests, they probably don't need to be visually tested. If they are important, then they should either have a demo in the documentation or have a story under the visual regression folder. This would reproduce the tradeoff we have in the main repository. The only difference is that we have no tools to easily display the visual regressions. |
import { XGrid, useGridApiRef } from '@material-ui/x-grid'; | ||
|
||
export default { | ||
title: 'regressions/Grid', |
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.
Not sure about this approach.
seems a bit segregated and I prefer to test some stories within their thematic folders. Maybe a prefix on the regressions tested stories would allow us to identify which one are tested?
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 could do a prefix. A bit harder to find and filter but they would be, as you said, in a thematic folder. No strong preferences.
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 guess that also the value of not using a semantic folder for the visual regressions. When you want to add a new regression test case, you don't need to care about it.
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.
For instance, here, now I need to ask:
@dtassone Where should this visual regression test be?
It's a real question, I don't know exactly where to put it, should I create a new section? I know it should be under the folder for the DataGrid, and not, say the WindowSplitter.
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'm trying something, we will see
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'm ok with the design, it does seem more intuitive. |
c68e226
to
99a855a
Compare
Co-authored-by: damien <damien.tassone@gmail.com>
A follow-up on #1025. Proposed design:
I have more or less reproduced the outlined style.