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

Improve UX when adding new records to a table with a NOT NULL column #775

Closed
seancolsen opened this issue Oct 29, 2021 · 10 comments · Fixed by #1385
Closed

Improve UX when adding new records to a table with a NOT NULL column #775

seancolsen opened this issue Oct 29, 2021 · 10 comments · Fixed by #1385
Assignees
Labels
affects: ux Related to user experience type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory
Milestone

Comments

@seancolsen
Copy link
Contributor

seancolsen commented Oct 29, 2021

Reproduce

  1. Set up a table with a NOT NULL column that doesn't have a default value.
  2. Click "New Record".
  3. Observe new row with yellow background, spinner that spins several seconds, finally a "Couldn't save changes" error.

image

I'm not sure what the expected behavior would be here. I think we need some more clarity around that from a UX perspective. @ghislaineguerin @pavish would you like to weight in?

@seancolsen seancolsen added type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory status: draft labels Oct 29, 2021
@mathemancer

This comment has been minimized.

@pavish
Copy link
Member

pavish commented Oct 29, 2021

@mathemancer I believe Sean's talking about adding a new record to a table that already has a column with a not null constraint.

@seancolsen Filling in the column with the not null constraint will save the record. From the db standpoint, We cannot create/update a record when it doesn't satisfy all the constraints associated with it.

As for the UX, we initially decided to show a red background with a message only on the particular cell, but considering that the whole record isn't saved, we found it better to show it on the whole record. However, we need to show a message on the cell using a popover, saying that it is mandatory, similar to how we do it for form validations.

Reference: Figma prototype for row warnings.

The prototype does not exactly cover cell level warnings. We can move ahead and improvise it during development

In cases where we improvise designs, we attach a video to the PR explaining the flow. @ghislaineguerin would review it and raise any red flags with regards to the UX. If it looks okay, we would merge it and then come back to it in the UI/UX sync.

@mathemancer

This comment has been minimized.

@seancolsen seancolsen changed the title Allow user to add new records in a table with a NOT NULL column Improve UX when adding new records to a table with a NOT NULL column Oct 29, 2021
@seancolsen
Copy link
Contributor Author

@pavish said:

Filling in the column with the not null constraint will save the record

I see. I'm confirming that now. I can add the new record, thankfully. But the current behavior does not make it clear to the user how to add the new record. I've updated the ticket title to reflect that this is a UX issue only.

@pavish pavish added the affects: ux Related to user experience label Oct 29, 2021
@pavish pavish added this to the [07] Working with Tables milestone Oct 29, 2021
@pavish
Copy link
Member

pavish commented Oct 29, 2021

@seancolsen As mentioned in the previous comment, we can improvise this during the development of the allow empty PR. My suggestion would be to show a small warning icon inside the cell and a validation popover when the user hovers it (This is similar to how it is on the figma prototype, except we show the warning only on the cell and not on the row).

@seancolsen
Copy link
Contributor Author

I'm marking this as blocked by #859 because we'll need more granular error info from the API before we're able to display a warning inside the cell

@seancolsen seancolsen added needs: unblocking Blocked by other work and removed status: draft labels Dec 1, 2021
@seancolsen seancolsen added ready Ready for implementation and removed needs: unblocking Blocked by other work labels Feb 1, 2022
@seancolsen
Copy link
Contributor Author

This should be ready to implement now that #859 is closed

@seancolsen
Copy link
Contributor Author

This is now blocked by #1051

@seancolsen seancolsen added needs: unblocking Blocked by other work and removed ready Ready for implementation labels Feb 7, 2022
@kgodey
Copy link
Contributor

kgodey commented Feb 14, 2022

@seancolsen Is this unblocked since we decided on client side validation (#1057)?

@seancolsen seancolsen added ready Ready for implementation and removed needs: unblocking Blocked by other work labels Feb 14, 2022
@seancolsen
Copy link
Contributor Author

@kgodey Indeed! Thanks for the follow-up. I've marked this ticket as unblocked now. Although I'll also mention that before resuming work on this ticket, I'd like to complete our (hopefully quick) planned synchronous discussion on "UX for failure to save a cell" at this Wednesday's meeting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: ux Related to user experience type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants