Skip to content
This repository has been archived by the owner on Nov 3, 2020. It is now read-only.

shallow copies are evil. Copy the DOM node properly. #126

Merged
merged 1 commit into from
Sep 3, 2020
Merged

shallow copies are evil. Copy the DOM node properly. #126

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 34 additions & 26 deletions client/src/components/HandMarkedPaperBallot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,28 +133,39 @@ class PostRenderBallotProcessor extends Handler {
if (pages.length % 2) {
const pagedjsPages = pages[0].pagesArea
if (pagedjsPages.lastChild) {
pagedjsPages.appendChild(pagedjsPages.lastChild.cloneNode(true))
const newLastPageElement = pagedjsPages.lastChild.cloneNode(
true
) as HTMLElement
pagedjsPages.appendChild(newLastPageElement)
pagedjsPages.setAttribute(
'style',
`--pagedjs-page-count:${pages.length + 1};`
)
const lastPage = pagedjsPages.lastChild! as Element
lastPage.id = `page-${pages.length + 1}`
lastPage.classList.remove('pagedjs_first_page', 'pagedjs_right_page')
lastPage.classList.add('pagedjs_left_page')
lastPage.setAttribute('data-page-number', `${pages.length + 1}`)
newLastPageElement.id = `page-${pages.length + 1}`
newLastPageElement.classList.remove(
'pagedjs_first_page',
'pagedjs_right_page'
)
newLastPageElement.classList.add('pagedjs_left_page')
newLastPageElement.setAttribute(
'data-page-number',
`${pages.length + 1}`
)
newLastPageElement.setAttribute('data-id', `page-${pages.length + 1}`)
Comment on lines +150 to +154
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason not to use the approach we had, i.e.

Suggested change
newLastPageElement.setAttribute(
'data-page-number',
`${pages.length + 1}`
)
newLastPageElement.setAttribute('data-id', `page-${pages.length + 1}`)
newLastPageElement.dataset.pageNumber = `${pages.length + 1}`
newLastPageElement.dataset.id = `page-${pages.length + 1}`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe, but the approach we had was via setAttribute so I didn't want to change it. I added the id one cause it was missing.

ReactDOM.render(
<BlankPageContent>
<Prose>
<p>This ballot page is intentionally blank.</p>
</Prose>
</BlankPageContent>,
lastPage.getElementsByClassName('pagedjs_page_content')[0]
newLastPageElement.getElementsByClassName('pagedjs_page_content')[0]
)

const newPage = { ...pages[pages.length - 1] }
newPage.element.dataset.pageNumber = `${pages.length + 1}`
newPage.element.dataset.id = `page-${pages.length + 1}`
const newPage = {
...pages[pages.length - 1],
element: newLastPageElement,
}
Comment on lines +164 to +167
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the actual fix, right? Everything else should be equivalent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should, yes. Trying to change as little as possible.


pages.push(newPage)
}
}
Expand All @@ -178,22 +189,19 @@ class PostRenderBallotProcessor extends Handler {
ballotType,
ballotId,
}: HMPBBallotMetadata = JSON.parse(qrCodeTarget.dataset.metadata ?? '')
ReactDOM.render(
<QRCode
level="H"
value={v1.encodeHMPBBallotPageMetadata(election, {
electionHash: electionHash.substring(0, 20),
ballotStyleId,
precinctId,
locales,
isTestMode,
pageNumber: parseInt(pageNumber!, 10),
ballotType,
ballotId,
})}
/>,
qrCodeTarget
)

const encoded = v1.encodeHMPBBallotPageMetadata(election, {
electionHash: electionHash.substring(0, 20),
ballotStyleId,
precinctId,
locales,
isTestMode,
pageNumber: parseInt(pageNumber!, 10),
ballotType,
ballotId,
})

ReactDOM.render(<QRCode level="H" value={encoded} />, qrCodeTarget)
}
})
}
Expand Down