Adding records

[pavish]

Fixes #197

Task list

• Add record action pane button
• Add record placeholder row
• Send request on adding record
• Change update behaviour to send post request if record is not created yet
• Handle record number for new records
• Handle deletion for new records
• Show indication that record is newly added
• Handle arrow key movement for negative index
• Handle conflict between reload-in-place functionality and new record iteration
• Show text to indicate record may change position

Demo

Mathesar - adding a record - Watch Video

Note

• We need to detail out the UX interactions and behaviour between operations like add/edit/delete/change-page/apply-sort,filter,group etc., when they are applied simultaneously.
• The columns API needs to provide information of whether the primary key column is an auto-incremented value or not. This will help us determine whether or not to allow editing of primary key values for new record addition.
• As discussed with @ghislaineguerin, the new records do not float. If they are allowed to float, and if there are multiple new records, they would take more space and cover other records behind them which is undesirable.

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.

[pavish]

@kgodey @ghislaineguerin This PR is ready for review again.

Short video describing the changes:

Google Chrome - Mathesar - Home - Watch Video

I'll create a design issue for the specifications/detailing required, as mentioned in previous comment.

[kgodey]

I played around with this locally and it looks good. Thanks for the video as well @pavish. We can open new issues if any bugs arise.

I'll leave it to @seancolsen to approve and merge.

[seancolsen]

My review process (listed here since I'm new):

• I skimmed the diff and did not identify any red flags at the code-level. Privately, I took note of some general questions about the code to ask Pavish on our next call, but I don't yet know the codebase deeply enough to spot any issues with the diff.

• I ran this PR locally and found a few things to suggest, listed below.

Suggested changes, highest priority first

• Use a better name for the button instead of "Record".

  I suggest "New row", but I think "Add row", "Add record", "New record", and "Insert" would also be fine. (I like "New row" best because it's consistent with preexisting buttons for "New table" and "New Schema".)

  Rationale: As-is, we're relying on the plus icon to convey functionality, which is poor accessibility. Without the icon, the functionality of a "Record" button is not clear to the user, especially because "Record" can be a verb too.

• Fix bug when deleting a record near the end of the table while adding a new record:

  To reproduce:

  1. Import patents.csv.
  2. Go to page 3.
  3. Click "+ Record".
  4. Select another row (e.g. 1390).
  5. Click "Delete 1 records"
  6. Expect to see either 1 green row at the bottom (persisting from our process of adding the new row) or 0 green rows at the bottom (because the table has been refreshed after deleting a row).
  7. Observe 2 green rows at the bottom in duplicate.

• Initiate adding when clicking on the "+" icon at the bottom of the table (within the row number column). I can click within other areas of that bottom row, but not on the icon itself, which is the spot I think users are most likely to click.

[pavish]

@seancolsen Regarding,

• Use a better name for the button instead of "Record".

This was followed based on our UX designs. I think we can add this to #722, so that @ghislaineguerin can decide on this.

• Fix bug when deleting a record near the end of the table while adding a new record

I am aware of this bug, I've mentioned it on the follow up design issue (#722), and also on the latest video. I've not handled it because it requires a bit of UX rethought.

I do agree we need to handle duplicates, however delete is an operation which reloads the table in-place without additional user intervention. This is different from other reloads like moving across pages, sorting etc., which require explicit user action. So, we cannot clear up the newly added records from the view during delete. We will need to think on the best way to handle this.

@ghislaineguerin Let us know if require further input from us. I've already added it as a point on the design issue, I'll also add @seancolsen's comment in the issue.

I think it would also make sense to have a dedicated issue for this.

Initiate adding when clicking on the "+" icon at the bottom of the table

Good point. I'll update the PR to handle this.

[kgodey]

The Add Record button name seems like a small enough issue that we can handle it here, @ghislaineguerin can you take a quick look at the discussion?

[ghislaineguerin]

@pavish Regarding @seancolsen comment on 'Add Record.' I agree with his points. I think we should use 'Add Row' as he suggests because when using this button from a view, we might have multiple records being added within the same row. I will update the design accordingly.

[seancolsen]

This may be splitting hairs, but just to clarify...

I think "Row" and "Record" are both acceptable words to use within the button. The change I'm requesting is that we add a verb like "New", "Add", or "Insert" before the noun. Any of those three verbs seem good to me, but "New" seems best because it's consistent with the buttons we already have for "New table" and "New Schema". And, in choosing between "Row" and "Record", I like "Row" because it's more concise. That's how I arrived at the suggestion of "New row".

[ghislaineguerin]

@seancolsen I see, I was just thinking about 'row' as I hadn't considered the action for adding within a view, and I think it makes more sense. As for the verb, maybe 'Insert' is better as it doesn't imply new or existing and would match our various scenarios.

[kgodey]

I think we probably want different words for tables and views. I like using "Record" for tables because the user can learn that tables contain a single record whereas views might have multiple records in a single row.

I like "New" or "Add" better than "Insert", it's shorter.

We will be doing another design/UX pass on the toolbar and general styling before we ship the alpha release, so I think we should just go with something for now. I suggest "New Record" or "Add Record". @ghislaineguerin what do you think?

[ghislaineguerin]

@kgodey ok, we might have to think about how we refer to the action in the context of views. But give that we'll rework most of it when designing the toolbar I think it's fine to have 'New Record' for now.

[mathemancer]

I think we probably want different words for tables and views. I like using "Record" for tables because the user can learn that tables contain a single record whereas views might have multiple records in a single row.

This is a great point. Seconded.

[pavish]

Sounds good. I'll update the PR with 'New Record' for now.

[seancolsen]

Looks good!