new GUI modal: tabulation confirmation page #812

artoonie · 2024-03-23T22:09:24Z

closes #679
closes #133 (but not yet #825)

Creates new GUI Tabulation Dialog (see corresponding issue)
Has a check to make sure the # of ballots didn't change between "Check Ballot Count" and "Tabulate."
Extensible so we can later has the entirety of the CVRs and check that there was no change whatsoever, not just in ballot counts
Extensible so we can pull additional metadata and show on the GUI before hitting tabulate

…confirm-num-ballots

yezr · 2024-05-08T15:19:30Z

Two things I noticed while looking over this

The progress bar hangs during tabulation. It starts out moving, then at some point during tabulation hangs. Here's a video.
Each run of tabulate takes a little bit longer than the previous one. Here's some splits I did with the Minneapolis Mayor test data. It got about 5 seconds longer each time I ran it. Not the end of the world, but might be worth looking into...

artoonie · 2024-05-08T19:08:33Z

Good catch! Both of these are present in 1.3.2 as well, though I agree this PR makes them more evident (especially the GUI lag).

I'll tackle the increase in timing first, then look at the GUI freezing. Since both exist outside of this PR, it may be useful to have separate tickets for them? But not super necessary.

yezr · 2024-05-08T19:16:08Z

I'll make new tickets. A P0 for the UI hanging #830. A P2 for the increased duration on consecutive runs #831

artoonie · 2024-05-08T22:13:11Z

Turns out both had the same underlying cause, and both are fixed in #832

tarheel

@HEdingfield since this is mostly UI, would you have time to review it?

…confirm-num-ballots

* make the log output faster, and add colors too * spruce up the style * Issue 830: prevent lagging * remove unused import * re-enable and fix word wrap * better warning color --------- Co-authored-by: Armin Samii <armin.samii@gmail.com>

yezr · 2024-06-03T23:51:40Z

can we make the Number of Ballots number formatted with commas on the thousands like 1,000 or 1,000,000

HEdingfield · 2024-06-04T00:59:36Z

can we make the Number of Ballots number formatted with commas on the thousands like 1,000 or 1,000,000

Sure, I'll handle this in my nit-handling commit shortly.

…confirm-num-ballots

artoonie · 2024-06-04T02:40:39Z

Done!

…ank()`; removes "please" from user comms per Google style; addresses nits, minor refactoring, and other clean-up.

config/spotbugs/exclude.xml

src/main/java/network/brightspots/rcv/Logger.java

HEdingfield · 2024-06-03T23:29:10Z

src/main/java/network/brightspots/rcv/Logger.java

+          item.setPrefWidth(listView.getWidth() - 50);
+
+          // Fix the label padding
+          setPadding(new Insets(0, 0, 0, 3));


Not sure how this looks on your system, but this leaves an awkwardly-large amount of spacing at the bottom of each line for me. Can we maybe try 0.5 or 1 instead? Actually, playing with these values doesn't seem to help the weird spacing. Could you please get to the bottom of this?

This was fixed here: 9b3b5f7

Not sure what git snafu happened here, but if you grab the latest, do you still see this?

Yep, still seeing this. :(

Ping on this. We can boot out to another issue if you'd like. CC: @yezr

Like Armin, I'm also not seeing the extra line spacing behavior with the latest code.

I think this is Windows-specific, but haven't had a chance to boot up my windows machine and check yet

Yep, this is a windows line ending issue. Fixed here, as the bug exists on develop too: #842

Excellent, thanks for fixing! I'll test that PR separately.

src/main/resources/network/brightspots/rcv/GuiConfigLayout.fxml

src/main/java/network/brightspots/rcv/GuiTabulateController.java

src/main/java/network/brightspots/rcv/TabulatorSession.java

src/main/java/network/brightspots/rcv/GuiConfigController.java

…ct location in Windows; fit and finish.

src/main/java/network/brightspots/rcv/GuiConfigController.java

src/main/java/network/brightspots/rcv/GuiTabulateController.java

HEdingfield · 2024-06-07T16:28:44Z

src/main/java/network/brightspots/rcv/Logger.java

+          item.setPrefWidth(listView.getWidth() - 50);
+
+          // Fix the label padding
+          setPadding(new Insets(0, 0, 0, 3));


Ping on this. We can boot out to another issue if you'd like. CC: @yezr

…confirm-num-ballots

HEdingfield

Great work, all set to merge!

src/main/java/network/brightspots/rcv/GuiTabulateController.java

HEdingfield · 2024-06-11T05:16:56Z

src/main/java/network/brightspots/rcv/Logger.java

+          item.setPrefWidth(listView.getWidth() - 50);
+
+          // Fix the label padding
+          setPadding(new Insets(0, 0, 0, 3));


Excellent, thanks for fixing! I'll test that PR separately.

new GUI modal: tabulation confirmation page

a4846c0

artoonie added the WIP label Mar 23, 2024

Armin Samii added 4 commits March 24, 2024 19:14

open in finder after tabulation; handle new file creation

743dc18

handle edge casess: trim CSV ranking, handle moved config file

22ed426

handle tabulation success/failure/in-progress states

6cb8d9f

lint

e18c5f7

artoonie changed the title ~~new GUI modal: tabulation confirmation page~~ WIP: new GUI modal: tabulation confirmation page Mar 31, 2024

exclude spotbugs error

fc56e93

artoonie force-pushed the feature/issue-679_confirm-num-ballots branch from f88126f to fc56e93 Compare March 31, 2024 16:34

artoonie marked this pull request as draft March 31, 2024 23:34

UI updates

256b816

artoonie force-pushed the feature/issue-679_confirm-num-ballots branch from 6b12f87 to 256b816 Compare April 13, 2024 21:00

Armin Samii added 2 commits April 21, 2024 17:30

Merge remote-tracking branch 'origin/develop' into feature/issue-679_…

e38b70b

…confirm-num-ballots

three-step gui tabulation

842fa95

artoonie force-pushed the feature/issue-679_confirm-num-ballots branch from 2c50723 to 842fa95 Compare April 21, 2024 23:32

artoonie force-pushed the feature/issue-679_confirm-num-ballots branch from d8def3b to 1f1f9d4 Compare April 29, 2024 19:23

don't allow change in ballot stats between loading and tabulating

3eca021

artoonie force-pushed the feature/issue-679_confirm-num-ballots branch from 1f1f9d4 to 3eca021 Compare April 29, 2024 19:54

artoonie removed the WIP label Apr 29, 2024

artoonie marked this pull request as ready for review April 29, 2024 20:26

artoonie changed the title ~~WIP: new GUI modal: tabulation confirmation page~~ new GUI modal: tabulation confirmation page Apr 30, 2024

gui improvements / timing

8fdb759

This was referenced May 8, 2024

UI hangs during tabulation #830

Closed

Tabulate takes ~5 seconds longer on consecutive runs #831

Closed

tarheel reviewed May 19, 2024

View reviewed changes

Merge remote-tracking branch 'origin/develop' into feature/issue-679_…

718efc3

…confirm-num-ballots

style updates

83b58eb

artoonie force-pushed the feature/issue-679_confirm-num-ballots branch from 3fa0575 to 83b58eb Compare May 30, 2024 17:34

HEdingfield self-requested a review June 3, 2024 22:41

Armin Samii added 2 commits June 3, 2024 22:33

Merge remote-tracking branch 'origin/develop' into feature/issue-679_…

ee0d49d

…confirm-num-ballots

comma-separate large number of ballots

2668ad4

Fixes unreliable usage of .isEmpty() for Strings in favor of `.isBl…

d594e4c

…ank()`; removes "please" from user comms per Google style; addresses nits, minor refactoring, and other clean-up.

HEdingfield requested changes Jun 4, 2024

View reviewed changes

Armin Samii added 3 commits June 4, 2024 11:45

address PR comments

0c00788

only pop up tabulate window if validation passes

bcb4c60

Update error message, other PR comments

85c4e51

artoonie force-pushed the feature/issue-679_confirm-num-ballots branch from 3e375d5 to 85c4e51 Compare June 4, 2024 16:16

Fixes bug with buttonOpenResultsClicked not opening window to corre…

82c25a1

…ct location in Windows; fit and finish.

yezr mentioned this pull request Jun 6, 2024

Multi-select UI log lines for copying #837

Closed

Armin Samii added 2 commits June 6, 2024 13:29

address PR comments

db9a51a

delete temp file on Tabulate Window close

0e84c5b

artoonie force-pushed the feature/issue-679_confirm-num-ballots branch from 9f10c5e to 0e84c5b Compare June 6, 2024 18:07

HEdingfield reviewed Jun 7, 2024

View reviewed changes

Nits.

beaf358

yezr mentioned this pull request Jun 7, 2024

Logger hanging in Windows when 'Open Log' button fails #840

Closed

PR updates

ffdf589

artoonie mentioned this pull request Jun 9, 2024

Feature/log output fixes #842

Merged

Merge remote-tracking branch 'origin/develop' into feature/issue-679_…

89c370e

…confirm-num-ballots

artoonie force-pushed the feature/issue-679_confirm-num-ballots branch from b1bfff7 to 89c370e Compare June 10, 2024 15:35

HEdingfield approved these changes Jun 11, 2024

View reviewed changes

artoonie merged commit 6171d29 into develop Jun 11, 2024
1 check passed

artoonie deleted the feature/issue-679_confirm-num-ballots branch June 11, 2024 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new GUI modal: tabulation confirmation page #812

new GUI modal: tabulation confirmation page #812

artoonie commented Mar 23, 2024 •

edited

Loading

yezr commented May 8, 2024

artoonie commented May 8, 2024

yezr commented May 8, 2024

artoonie commented May 8, 2024

tarheel left a comment

yezr commented Jun 3, 2024

HEdingfield commented Jun 4, 2024

artoonie commented Jun 4, 2024

HEdingfield Jun 3, 2024

artoonie Jun 4, 2024

HEdingfield Jun 6, 2024

HEdingfield Jun 7, 2024

yezr Jun 7, 2024

artoonie Jun 7, 2024

artoonie Jun 9, 2024

HEdingfield Jun 11, 2024

HEdingfield Jun 7, 2024

HEdingfield left a comment

HEdingfield Jun 11, 2024

new GUI modal: tabulation confirmation page #812

new GUI modal: tabulation confirmation page #812

Conversation

artoonie commented Mar 23, 2024 • edited Loading

yezr commented May 8, 2024

artoonie commented May 8, 2024

yezr commented May 8, 2024

artoonie commented May 8, 2024

tarheel left a comment

Choose a reason for hiding this comment

yezr commented Jun 3, 2024

HEdingfield commented Jun 4, 2024

artoonie commented Jun 4, 2024

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

HEdingfield left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artoonie commented Mar 23, 2024 •

edited

Loading