Polish CVR import and export UIs #4047

arsalansufi · 2023-10-06T03:12:50Z

Easiest reviewed by commit

Overview

Issue links: #3961 + #3967

This PR polishes the CVR import and export UIs. More specifically, it:

Removes references to singular CVR files, since CVR exports now consist of multiple files
Makes copy and UX more consistent and concise
Doesn't allow closing saving/loading modals by clicking outside them
Removes uses of the deprecated Prose component and non-libs/ui Loading component

It also restores the ability to manually select a CVR file/directory for import. Something that I'd previously punted on (#3956 (comment)).

Demo Video

VxCentralScan export

vxcentralscan.mov

VxScan export

vxscan.mov

VxAdmin import

vxadmin.mov

vxadmin-2.mov

Testing

Updated automated tests
Tested the UIs manually

Checklist

~~I have added logging where appropriate to any new user actions, system updates such as file reads or storage writes, or errors introduced~~ N/A
I have added a screenshot and/or video to this PR to demo the change
I have added the "user_facing_change" label to this PR to automate an announcement in #machine-product-updates

arsalansufi · 2023-10-06T03:13:17Z

apps/admin/backend/src/app.ts

-      const importResult = await importCastVoteRecords(store, input.path);
+      const exportDirectoryPath =
+        // For manual export selection, users must select the contained metadata file as a proxy
+        // for the export directory since the UI doesn't support directory selection


Closing the loop on #3956 (comment)!

Whereas you could previously select the top-level cast-vote-record-report.json, you can now select the top-level metadata.json. I couldn't find an easy way to support directory selection. Though non-ideal, we really only use this feature in dev. If an election official were to have to use it, I'd imagine it be in a context where we were guiding them.

🙏 🙏 🙏 🙏 🙏 🙏 🙏 🙏 🙏 🙏

arsalansufi · 2023-10-06T03:18:20Z

apps/admin/frontend/src/components/confirm_removing_file_modal.tsx

@@ -70,7 +70,7 @@ export function ConfirmRemovingFileModal({

  return (
    <Modal
-      centerContent


Seems like we more often left-align our modals than center-align them, so I've made that consistent for CVR modals.

arsalansufi · 2023-10-06T03:19:33Z

apps/admin/frontend/src/components/import_cvrfiles_modal.tsx

-            The selected file was ignored as a duplicate of a previously loaded
-            file.
+            The selected export was ignored as a duplicate of a previously
+            loaded export.


Because exports are now (and have for a while been) directories instead of single files, I've moved away from "file" as the descriptor for a CVR export and shifted to "export"

arsalansufi · 2023-10-06T03:24:19Z

apps/admin/frontend/src/components/import_cvrfiles_modal.tsx

+          </P>
+        );
+      }
+      return <P>The CVRs in the selected export were successfully loaded.</P>;


Aside from better handling singular vs. plural cases, I've added content for the basic, no-duplicates success case. We used to have no content for the success case, just a title. Which made the modal look a bit funny IMO. I've added content for consistency with the other cases.

Before / After

arsalansufi · 2023-10-06T03:26:43Z

apps/admin/frontend/src/components/import_cvrfiles_modal.tsx

@@ -310,6 +315,10 @@ export function ImportCvrFilesModal({ onClose }: Props): JSX.Element | null {
    );
  }

+  if (currentState.state === 'loading') {
+    return <Modal content={<Loading>Loading CVRs</Loading>} />;


Note that I'm intentionally not specifying an onOverlayClick prop for saving/loading modals so that clicks outside them don't accidentally close them. This came up a few times during the SLI test as a source of confusion. Clicking outside the modal would dismiss it, but the relevant operation would continue behind the scenes, leaving the machine sluggish without any clear indication as to why it was sluggish.

arsalansufi · 2023-10-06T03:30:42Z

apps/admin/frontend/src/components/import_cvrfiles_modal.tsx

-          this USB drive. Save CVR files to this USB drive from the scanner.
-          Optionally, you may manually select files to load.
+          No new {headerModeText.toLowerCase()} CVR exports were automatically
+          found on this USB drive.


Erring on the side of more concise text since I know longer text often isn't read

arsalansufi · 2023-10-06T03:30:58Z

apps/admin/frontend/src/screens/tally_screen.tsx

+            >
+              Clear All Tallies and Results
+            </Button>
+          </P>


The spacing for this element is off without a <P>

I noticed this on my branch yesterday, thanks for addressing!

arsalansufi · 2023-10-06T03:31:06Z

apps/central-scan/frontend/src/components/export_results_modal.tsx

@@ -81,12 +81,9 @@ export function ExportResultsModal({ onClose }: Props): JSX.Element {
    if (usbDriveStatus === 'ejected') {
      return (
        <Modal
-          title="CVRs Saved"
+          title="USB Drive Ejected"


I've made the CVR export modals in VxCentralScan and VxScan consistent. Prior, there were a number of miscellaneous inconsistencies both within and between the modals. I could of course set up a shared component but decided to punt on that for time's sake

Enough possible divergence that keeping them separate now seems safer.

arsalansufi · 2023-10-06T03:33:00Z

apps/central-scan/frontend/jest.config.js

-      functions: 70,
-      lines: 81,
+      functions: 69,
+      lines: 80,


These dropped because I got rid of the one-off Prose and Loading components within apps/central-scan/frontend

adghayes

LGTM! Exciting to get to all these polish items.

Two additional polish items I'm thinking of after looking through your PR, both of which predate your work:

Nit: comma-format the numbers in the Load CVRs Modal:

Allow deleting CVR files individually. See VxAdmin: Allow Deleting CVR Imports Individually #4048. We can have discussion about this on the issue page or take it to Slack.

adghayes · 2023-10-06T09:10:15Z

apps/admin/backend/src/app.ts

-      const importResult = await importCastVoteRecords(store, input.path);
+      const exportDirectoryPath =
+        // For manual export selection, users must select the contained metadata file as a proxy
+        // for the export directory since the UI doesn't support directory selection


🙏 🙏 🙏 🙏 🙏 🙏 🙏 🙏 🙏 🙏

adghayes · 2023-10-06T09:12:14Z

apps/admin/frontend/src/components/import_cvrfiles_modal.tsx

@@ -310,6 +315,10 @@ export function ImportCvrFilesModal({ onClose }: Props): JSX.Element | null {
    );
  }

+  if (currentState.state === 'loading') {
+    return <Modal content={<Loading>Loading CVRs</Loading>} />;


adghayes · 2023-10-06T09:13:26Z

apps/admin/frontend/src/screens/tally_screen.tsx

+            >
+              Clear All Tallies and Results
+            </Button>
+          </P>


I noticed this on my branch yesterday, thanks for addressing!

adghayes · 2023-10-06T09:14:12Z

apps/central-scan/frontend/src/components/export_results_modal.tsx

@@ -81,12 +81,9 @@ export function ExportResultsModal({ onClose }: Props): JSX.Element {
    if (usbDriveStatus === 'ejected') {
      return (
        <Modal
-          title="CVRs Saved"
+          title="USB Drive Ejected"


Enough possible divergence that keeping them separate now seems safer.

- Remove references to singular CVR files, since CVR exports now consist of multiple files - Make copy and UX more consistent and concise - Don't allow closing saving/loading modals by clicking outside them - Remove uses of deprecated Prose component and non-libs/ui Loading component

To account for removal of <Prose> and <Loading>

Also clear error message on subsequent backup attempts

arsalansufi · 2023-10-06T15:41:42Z

@adghayes Good catch re number formatting inside the modal! Added in ef63b3f:

And re deleting individual CVR imports, makes a lot of sense to me! Continued the discussion on the linked issue.

arsalansufi added the user_facing_change label Oct 6, 2023

arsalansufi commented Oct 6, 2023

View reviewed changes

arsalansufi requested a review from adghayes October 6, 2023 03:35

arsalansufi marked this pull request as ready for review October 6, 2023 03:35

arsalansufi requested a review from a team as a code owner October 6, 2023 03:35

arsalansufi requested review from carolinemodic and removed request for a team and carolinemodic October 6, 2023 03:35

adghayes approved these changes Oct 6, 2023

View reviewed changes

Base automatically changed from arsalan/misc-cleanup to main October 6, 2023 15:07

arsalansufi requested a review from carolinemodic as a code owner October 6, 2023 15:07

arsalansufi removed the request for review from carolinemodic October 6, 2023 15:08

arsalansufi force-pushed the arsalan/ux-polish branch from 4063b4f to 2c339d3 Compare October 6, 2023 15:10

arsalansufi added 6 commits October 6, 2023 11:18

Restore ability to manually select a CVR file/directory for import

814ab01

Update tests

c5462b8

Adjust VxCentralScan frontend code coverage thresholds

e926d13

To account for removal of <Prose> and <Loading>

Make sure saving backup on VxCentralScan doesn't crash app when no USB

35fff0c

Also clear error message on subsequent backup attempts

Update integration tests

e3020c0

arsalansufi force-pushed the arsalan/ux-polish branch from 2c339d3 to e3020c0 Compare October 6, 2023 15:18

Format large numbers with commas, in CVR import modal

ef63b3f

arsalansufi merged commit dc86724 into main Oct 6, 2023

arsalansufi deleted the arsalan/ux-polish branch October 6, 2023 15:44

arsalansufi mentioned this pull request Oct 23, 2023

Update variable names, etc., to no longer refer to CVRs as single files #4097

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Polish CVR import and export UIs #4047

Polish CVR import and export UIs #4047

arsalansufi commented Oct 6, 2023 •

edited

Loading

arsalansufi Oct 6, 2023

adghayes Oct 6, 2023

arsalansufi Oct 6, 2023

arsalansufi Oct 6, 2023

arsalansufi Oct 6, 2023

arsalansufi Oct 6, 2023

adghayes Oct 6, 2023

arsalansufi Oct 6, 2023

arsalansufi Oct 6, 2023

adghayes Oct 6, 2023

arsalansufi Oct 6, 2023

adghayes Oct 6, 2023

arsalansufi Oct 6, 2023

adghayes left a comment

adghayes Oct 6, 2023

adghayes Oct 6, 2023

adghayes Oct 6, 2023

adghayes Oct 6, 2023

arsalansufi commented Oct 6, 2023

Polish CVR import and export UIs #4047

Polish CVR import and export UIs #4047

Conversation

arsalansufi commented Oct 6, 2023 • edited Loading

Overview

Demo Video

Testing

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adghayes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arsalansufi commented Oct 6, 2023

arsalansufi commented Oct 6, 2023 •

edited

Loading