UI for adding multi-column unique constraints

[seancolsen]

Fixes #531

Demos

• Demo 1 -- Second modal for confirming delete
• Demo 2 -- Second modal for adding constraint
• Demo 3 -- Form for adding new unique constraints

Changes

• Modify table constraints dialog to allow users to add new unique constraints.

• Allow multiple modals to display at once -- so that we can display the form for adding a new unique constraint on top of the modal which displays all constraints.

• Change the UI for confirming the deletion of a constraint to use our modal confirmation system instead of the inline sliding confirmation. This reduces code complexity and leads to a more consistent UI.

• Split Modal component into

  • IndependentModal -- a simple base component for use-cases where we don't have a controller. (In Mathesar, I don't think we'll ever want to use this component, but people using the component library might.)
  • ControlledModal -- a higher-level component that composes IndependentModal and requires a ModalController

• Refactor AddEditSchema and RenameTableModal into simpler components that both compose ModalTextInputForm (a new component) to reduce code duplication and ensure consistent UI.

Review tips

• Play with the process of adding new unique constraints and deleting constraints.
• Play with the modal system in storybook
• Play with other modals in the app, since I did some refactoring which touches all modals.
  • Confirmations used when deleting anything, e.g. delete table
  • Create/edit schema
  • Rename table

Issues

• The Form and FormField components (which exist only for styling) might overlap a bit with the form-builder system in terms of intended purpose. I haven't yet looked into this more deeply.

• Modal z index stacking is very primitive at the moment. All modals have the same z index, which means they are layered in the order in which they are first opened. Technically, this could lead to bugs, but it's a little hard to imagine a real-world scenario that would trigger the bug. You'd need to begin with no modals, then open modal A, then open modal B, then re-open modal A again (even though it's already open). At that point, you'd expect modal A to be re-layered on top of modal B, but due to its z-index being identical to modal B it would remain on bottom. The data structure already supports this re-layering correctly. It would just be a little extra work to set the z-index based on the stacking order we're storing. That's totally feasible. I just haven't yet taken the time to do it. We could take that up later if this whole multi-modal approach actually seems viable.

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the master branch of the repository
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[codecov-commenter]

Codecov Report

Merging #1005 (a0dce29) into 497_rename_column_b (9d30e3b) will not change coverage.
The diff coverage is n/a.

@@                 Coverage Diff                  @@
##           497_rename_column_b    #1005   +/-   ##
====================================================
  Coverage                93.27%   93.27%           
====================================================
  Files                       95       95           
  Lines                     3463     3463           
====================================================
  Hits                      3230     3230           
  Misses                     233      233           

Flag	Coverage Δ
pytest-backend	93.27% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d30e3b...a0dce29. Read the comment docs.

[seancolsen]

@pavish this is ready for review now.

@ghislaineguerin I'd also like you to look at this, since the UX is different than what we initially discussed. I explain a bit about this divergence in the "Demo 2" video. The direction I'm going here is to display multiple modals on top of each other. Pavish and I chatted a bit about this approach on a call. He expressed some reservations about it but figured it would be worth exploring.

Pavish, we initially talked about hiding the lower modal in some cases when the upper modal is displaying. After giving this some more thought I decided to always show the lower modal. For one thing it's simpler to implement. But also I prefer it from a UX perspective too. I think that placing the lower modal under the overlay sufficiently communicates the UI state to the user, making it clear that they can only interact with the top-most modal. It also adds additional clarity that the steps they took within the lower modal have persisted.

[ghislaineguerin]

@seancolsen, your video provides a reasonable explanation for taking this approach. I don't feel that the modal on top of the modal is bad. If they are different enough, as you describe, it makes sense to overlap them.

[kgodey]

I'm not the biggest fan of the double modal UX, but I think it's fine for now. I think in the long run, perhaps we should show table constraints in a separate pane or page rather than a modal. Then we can have deletion confirmation etc. be a modal without the double modals.

[seancolsen]

@kgodey

I think in the long run, perhaps we should show table constraints in a separate pane or page rather than a modal.

Yeah I think that's definitely worth considering. We could also consider separating the DDL and DML operations for each table into separate tabs. Many DB GUIs I've worked with behave this way. You get one tab that lists all the columns, constraints, indexes, allowing you to add/update/delete those things. Then separately there's a different tab which lists all the rows in the table. Right now that's not super easy to do without some refactoring to the tabs system.

I think it's definitely worth trying to avoid multiple modals and we should be keeping this in mind going forward. If we're inclined to put something in a modal we should always be asking "is there anything the user might want to do inside this modal that would require another modal?".

[kgodey]

We could also consider separating the DDL and DML operations for each table into separate tabs. Many DB GUIs I've worked with behave this way. You get one tab that lists all the columns, constraints, indexes, allowing you to add/update/delete those things. Then separately there's a different tab which lists all the rows in the table. Right now that's not super easy to do without some refactoring to the tabs system.

I think in Mathesar's use case, we could tuck DML operations into a separate page. I think users will be using Mathesar for DDL far more often than DML. We can figure out a good pattern for this when we start working on the Data Modeling milestone cc @ghislaineguerin.

I think it's definitely worth trying to avoid multiple modals and we should be keeping this in mind going forward. If we're inclined to put something in a modal we should always be asking "is there anything the user might want to do inside this modal that would require another modal?".

Agreed, also cc @ghislaineguerin.

[pavish]

@seancolsen The PR looks good to me.

I do have my reservations of allowing opening multiple modals but I think we can come back to this after our alpha release. The discussion on this PR regarding a separate page for DML operations makes sense to me.

There are some action items:

1. Update this PR to resolve merge conflicts.
2. Stack this PR on top of Implement renaming a column #976. I'd like to merge Implement renaming a column #976 first.
3. The Form and FormField components focus on composing forms using slots, whereas the form-builder component focuses on generating a form using a json config. Even though there is a clear different in usecase, they share the same component name and css classes. I'd like to avoid that. I think it makes more sense to keep the names of these components the same. I'll change the names of form-builder components as part of Type configuration for Text, Number, Boolean, and all mathesar types with single associated db type #926

Update:
The changes for 3 is done in deec945...45029a1.

[seancolsen]

@pavish I've resolved merge conflicts and stacked this on top of #976. Back to you now.