-
Notifications
You must be signed in to change notification settings - Fork 7
refactor: clarify the tally report types #791
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great.
<NoWrap>Tally Report</NoWrap> | ||
</h1> | ||
<p> | ||
This report should be <strong>{reportPurpose}</strong>. | ||
</p> | ||
<p> | ||
{isPollsOpen ? 'Polls Closed' : 'Polls Opened'} and report printed at:{' '} | ||
{isPollsOpen ? 'Polls Closed' : 'Polls Opened'} and report printed at: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please confirm that there is still a space after the :
in the rendered content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out this was due to a bug that's now fixed: jsx-eslint/eslint-plugin-react#2437. I'll update eslint-plugin-react
in your PR.
@@ -54,14 +60,14 @@ const PrecinctTallyReport = ({ | |||
return ( | |||
<Report> | |||
<h1> | |||
<NoWrap>{precinct.name}</NoWrap> <NoWrap>{election.title}</NoWrap>{' '} | |||
<NoWrap>{precinct.name}</NoWrap> <NoWrap>{election.title}</NoWrap> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please confirm that there is still a space between the election title and "Tally Report" in the rendered content.
Sorry to make you rebase… I was just doing what you asked. 😝 |
This changes the types for the candidate and yesno tallies from arrays into objects. For candidate contests, it splits the former single array that contained both write-ins and ballot candidates into separate properties on an object. For yesno contests, instead of a 2-element tuple we now have an object with "yes" and "no" properties. I think this is simpler and requires less casting. It also adds a few assertions for situations which shouldn't happen but otherwise might silently fail.
c3e94c8
to
f6c684c
Compare
No problem! Glad to see the improvements. |
This changes the types for the candidate and yesno tallies from arrays into objects. For candidate contests, it splits the former single array that contained both write-ins and ballot candidates into separate properties on an object. For yesno contests, instead of a 2-element tuple we now have an object with "yes" and "no" properties.
I think this is simpler and requires less casting. It also adds a few assertions for situations which shouldn't happen but otherwise might silently fail.