Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support a large top margin on bmd_paper_ballot #3936

Merged
merged 4 commits into from
Sep 12, 2023

Conversation

kshen0
Copy link
Contributor

@kshen0 kshen0 commented Sep 6, 2023

Overview

#3872

  • Adds an optional 1.75in margin to the BMD ballot component so the top of the ballot isn't cut off when presenting the ballot for validation
  • Changes the implementation of the print state
    • The old implementation triggered the scan state too early. When printing longer ballots, the paper would advance such that paperInOutput would return true in the middle of a print, causing the paper to be pulled into the scanner before printing was done.

Demo Video or Screenshot

Before
IMG_4236

After
IMG_4238
IMG_4237

Testing Plan

  • Added test to BMD ballot component

Checklist

  • I have added logging where appropriate to any new user actions, system updates such as file reads or storage writes, or errors introduced.
  • 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

@kshen0 kshen0 force-pushed the kevin/vxmarkscan-add-margin-to-top-of-ballot branch from 759e1b3 to b072678 Compare September 11, 2023 22:59
@kshen0 kshen0 force-pushed the kevin/vxmarkscan-add-margin-to-top-of-ballot branch from b072678 to ec0f083 Compare September 11, 2023 23:21
@kshen0 kshen0 requested a review from jonahkagan September 11, 2023 23:26
@kshen0 kshen0 marked this pull request as ready for review September 11, 2023 23:26
@kshen0 kshen0 requested a review from a team as a code owner September 11, 2023 23:26
@kshen0 kshen0 requested review from adghayes and removed request for a team September 11, 2023 23:26
@@ -275,7 +281,10 @@ export function BmdPaperBallot({

return withPrintTheme(
<Ballot aria-hidden>
<Header>
<Header
className={largeTopMargin ? 'large-top-margin' : undefined}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to use a class here vs just passing a prop to the Header styled component? (that's the pattern I usually see in our code for parameterized styling)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't know about conditional styling with the styled-components lib!

id: 'printBallot',
src: (context, event) => {
// Need to hint to Typescript that we want the 'VOTER_INITIATED_PRINT' event in our union type of events
if ('pdfData' in event) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do assert(event.type === 'VOTER_INITIATED_PRINT') to make this a bit more concise. Our assert function will hint the type checker

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

display: flex;
flex-direction: row;
align-items: center;
border-bottom: 0.2em solid #000;
margin-top: ${(p) => (p.largeTopMargin ? '1.75in' : undefined)};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the right way to fallback, but I explicitly don't want to overwrite margin with 0 in the case where largeTopMargin is falsy.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it works, then seems fine to me!

@kshen0 kshen0 force-pushed the kevin/vxmarkscan-add-margin-to-top-of-ballot branch from 95485da to d0be2c1 Compare September 12, 2023 19:01
target: 'scanning',
},
},
pollPaperStatus(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nb. pollPaperStatus() is still needed here to inform the top-level handler for jammed.

@kshen0 kshen0 force-pushed the kevin/vxmarkscan-add-margin-to-top-of-ballot branch from d0be2c1 to 19d6b35 Compare September 12, 2023 19:18
@kshen0 kshen0 force-pushed the kevin/vxmarkscan-add-margin-to-top-of-ballot branch from 19d6b35 to 46c286e Compare September 12, 2023 19:23
@kshen0 kshen0 merged commit c2ba8bc into main Sep 12, 2023
@kshen0 kshen0 deleted the kevin/vxmarkscan-add-margin-to-top-of-ballot branch September 12, 2023 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants