-
-
Notifications
You must be signed in to change notification settings - Fork 321
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
Improve UX for new row with NOT NULL constraint #1385
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
Ok, the E2E test failure here is actually a good catch. Thanks, Playwright! I need to spend some more time on this, so I'm sending it back to draft state. |
d1acde6
to
a445a6e
Compare
I pushed another commit to fix the bug that the E2E test caught. It brought to light an interesting edge case which warrants its own demo. Please also watch this video when reviewing: |
…775_new_row_with_not_null
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.
@seancolsen Nice work on this!
I tested a few scenarios and the UX looks good to me. There are some scenarios that are okay with me but I'm not sure on how the target users might perceive it.
For eg., For checkbox, when DEFAULT is displayed and the user clicks on the cell, it toggles the value and the checkbox is then displayed. This seems reasonable to me, but this is one of the areas where we should try to get the user's opinions when possible after the alpha.
Fixes #775
Fixes #1362
Demo
https://www.loom.com/share/b8da99a10eec4719b71c4ade93102c85
Checklist
Update index.md
).master
branch of the repositoryDeveloper Certificate of Origin
Developer Certificate of Origin