sigit/sigit.io
7
2
Fork 0
Code Issues 51 Pull Requests 3 Actions Packages Projects 1 Releases Wiki Activity

issue-99-signing-page-refactor #124

Closed
eugene wants to merge 0 commits from issue-99-signing-page-refactor into issue-99
pull from: issue-99-signing-page-refactor
merge into: sigit:issue-99
Conversation 1 Commits - Files Changed -
eugene commented 2024-08-02 11:51:50 +00:00
Owner
Copy Link

This is a chunky PR, so unfortunately it has to be accompanied by a wall of text below. Going forward, will be sticking to slimmer PRs.

Key objectives of this PR:

  1. When the user goes to sign a document, they should see a rendered PDF page with the annotations/marks that they need to fill in highlighted on the document.
  2. A text box should be displayed at the bottom that they can enter text data into. The text that they enter should be displayed in the corresponding mark on the document.
  3. Once the user goes through each mark/field, they should be prompted to complete signing by signing the document.
  4. The completed marks and their values will be saved in the DocSignatures part of Meta.
  5. Once the signing process is complete for all users, the user should be able to export that Sigit as a zip file that includes the meta.json info and the fullt rendered PDF.

Changes included in the PR.

  • Mark interface wsa changed to include all relevant information for each mark, including pdfFileHash and a unique ID. Instead of a nested data structure, we now have an array of Mark objects which makes it easy to iterate over them without always transforming Objects back into Arrays and vice versa.

  • CurrentUserMark was added as an internal type to represent Marks that have been processed by Sigit for filling in and displaying on the document. CurrentUserMark contains all the relevant information, such as whether the mark is complete and whether is is a last mark - required for efficient rendering and user flow.
    (Happy to chat about it further. If we feel that it's a redundant type, it should be easy to remove, since it's an internal type only)

  • Third, I've added the following components to render Marked PDFs:

PDFMarking - top level component that keeps track of pdf files, pages, marks, and current form values
PdfView - Manages all pdf files
PdfItem - Displays each file
PdfPageItem- Displays each page of each file
PdfMarkItem - Renders each mark

In PdfMarkItem, userMark represents each mark to be rendered, while selectedMark and its corresponding selecredMarkValue represents the mark that the user is currently editing.

The reason I went for a nested structure is because I thought that the sign/index handler is already quite big. At the top level, sign/index can control which part of the signing process needs to be displayed at any point. isReadyToSign is the flat that sign/index can use to decide whether to display PDF Marking and Rendering components or the Signing Component.

  • MarkFormField is the component that manages the bottom form part of the marking process.

  • VerifyPage was changed to include the "export" button. Once the document is fully signed, the user can export it. The PDF will be generated, alongside the meta.json file, and saved on the user computer.

  • utils/mark contains utility methods related to dealing with Marks and CurrentUserMarks

  • utils/pdf contains helper functions and shared functionality related to dealing with PDFs

  • utils/sign contains shared methods related to signing

What I have tested

  • I was able to create a brand new sigit, add one or more text fields, assign them to myself, then add text to each field, sign the sigit, and download the zip pdf.
  • I was able to follow the same flow with with two users, although on a couple of occasions I had problems with decrypting sigits. I suspect the error has to do with some kind of extension malfunction.

Current limitations

  • I haven't done a lot of UI work yet, especially with new designs.
  • There is still some commented out code, so this PR should be seen as work-in-progress still
  • Need to test various flows, i.e. Online flow, Navigation flow, and offline flow. Probably more work will be required to wire up each function. As a suggestion, we could slim down sign/index to effectively only control which components should be rendering, and move as much processing logic to its related components.
  • The export button in the verify/index component is definitely not in the right place, but as mentioned above, I was primarily focusong on functionality.
  • The export button in the sign/index doesn't currently work properly. This is probably the most straightforward next bit of work.
  • Finally, as we discussed below, the current PDF generating process includes
    a) transforming PDF to PNG, which involves some scaling issues and other compatibility issues (like, font size)
    b) storing Mark coordinates separately
    c) storing completed marks separately
    d) Always converting PDF to PNG before rendering it.
    f) Creating a brand new PDF when we generate it at the end of the process and adding annotations using a completely separate tool.

I would be quite keen to explore if pdf-lib or other sinmilar library (I'm not wedded to pdf lib) can streamline the process. If we can embed the fields on the file itself, then we can actually remove the Mark data structures completely, unless I'm missing anything.

Please feel free to pick this PR apart mercilessly if you feel that something can be done better! I'm still finding my way around the repo, so could have missed some crucial bits.

This is a chunky PR, so unfortunately it has to be accompanied by a wall of text below. Going forward, will be sticking to slimmer PRs. Key objectives of this PR: 1. When the user goes to sign a document, they should see a rendered PDF page with the annotations/marks that they need to fill in highlighted on the document. 2. A text box should be displayed at the bottom that they can enter text data into. The text that they enter should be displayed in the corresponding mark on the document. 3. Once the user goes through each mark/field, they should be prompted to complete signing by signing the document. 4. The completed marks and their values will be saved in the DocSignatures part of Meta. 5. Once the signing process is complete for all users, the user should be able to export that Sigit as a zip file that includes the meta.json info and the fullt rendered PDF. Changes included in the PR. - `Mark` interface wsa changed to include all relevant information for each mark, including `pdfFileHash` and a unique `ID`. Instead of a nested data structure, we now have an array of Mark objects which makes it easy to iterate over them without always transforming Objects back into Arrays and vice versa. - `CurrentUserMark` was added as an internal type to represent Marks that have been processed by Sigit for filling in and displaying on the document. `CurrentUserMark` contains all the relevant information, such as whether the mark is complete and whether is is a last mark - required for efficient rendering and user flow. (Happy to chat about it further. If we feel that it's a redundant type, it should be easy to remove, since it's an internal type only) - Third, I've added the following components to render Marked PDFs: `PDFMarking` - top level component that keeps track of pdf files, pages, marks, and current form values `PdfView` - Manages all pdf files `PdfItem` - Displays each file `PdfPageItem`- Displays each page of each file `PdfMarkItem` - Renders each mark In `PdfMarkItem`, `userMark` represents each mark to be rendered, while `selectedMark` and its corresponding `selecredMarkValue` represents the mark that the user is currently editing. The reason I went for a nested structure is because I thought that the `sign/index` handler is already quite big. At the top level, `sign/index` can control which part of the signing process needs to be displayed at any point. `isReadyToSign` is the flat that `sign/index` can use to decide whether to display PDF Marking and Rendering components or the Signing Component. - `MarkFormField` is the component that manages the bottom form part of the marking process. - `VerifyPage` was changed to include the "export" button. Once the document is fully signed, the user can export it. The PDF will be generated, alongside the meta.json file, and saved on the user computer. - `utils/mark` contains utility methods related to dealing with `Marks` and `CurrentUserMarks` - `utils/pdf` contains helper functions and shared functionality related to dealing with PDFs - `utils/sign` contains shared methods related to signing **What I have tested** - I was able to create a brand new sigit, add one or more text fields, assign them to myself, then add text to each field, sign the sigit, and download the zip pdf. - I was able to follow the same flow with with two users, although on a couple of occasions I had problems with decrypting sigits. I suspect the error has to do with some kind of extension malfunction. **Current limitations** - I haven't done a lot of UI work yet, especially with new designs. - There is still some commented out code, so this PR should be seen as work-in-progress still - Need to test various flows, i.e. Online flow, Navigation flow, and offline flow. Probably more work will be required to wire up each function. As a suggestion, we could slim down `sign/index` to effectively only control which components should be rendering, and move as much processing logic to its related components. - The export button in the `verify/index` component is **definitely** not in the right place, but as mentioned above, I was primarily focusong on functionality. - The export button in the `sign/index` doesn't currently work properly. This is probably the most straightforward next bit of work. - Finally, as we discussed below, the current PDF generating process includes a) transforming PDF to PNG, which involves some scaling issues and other compatibility issues (like, font size) b) storing Mark coordinates separately c) storing completed marks separately d) Always converting PDF to PNG before rendering it. f) Creating a brand new PDF when we generate it at the end of the process and adding annotations using a completely separate tool. I would be quite keen to explore if `pdf-lib` or other sinmilar library (I'm not wedded to pdf lib) can streamline the process. If we can embed the fields on the file itself, then we can actually remove the Mark data structures completely, unless I'm missing anything. Please feel free to pick this PR apart _mercilessly_ if you feel that something can be done better! I'm still finding my way around the repo, so could have missed some crucial bits.
eugene added 11 commits 2024-08-02 11:51:50 +00:00
m approved these changes 2024-08-02 15:43:51 +00:00
src/components/DrawPDFFields/style.module.scss Outdated
@ -62,2 +62,2 @@
width: 40px;
height: 40px;
//width: 40px;
//height: 40px;
m commented 2024-08-02 15:16:33 +00:00
Owner
Copy Link

I suggest removing the commented code or providing more details in a comment as to why we should keep it.

I suggest removing the commented code or providing more details in a comment as to why we should keep it.
👍 1
eugene marked this conversation as resolved
src/pages/create/index.tsx Outdated
@ -519,6 +526,8 @@ export const CreatePage = () => {
title
}
console.log('content: ', content)
m commented 2024-08-02 15:35:19 +00:00
Owner
Copy Link

Please remove the console.log

Please remove the console.log
eugene marked this conversation as resolved
src/pages/sign/index.tsx Outdated
@ -296,10 +319,17 @@ export const SignPage = () => {
}
}
console.log('processed files: ', files);
m commented 2024-08-02 15:36:43 +00:00
Owner
Copy Link

Please remove the console.log

Please remove the console.log
eugene marked this conversation as resolved
src/pages/sign/index.tsx Outdated
@ -434,6 +455,8 @@ export const SignPage = () => {
}
}
console.log('processed files: ', files);
m commented 2024-08-02 15:38:19 +00:00
Owner
Copy Link

Please remove the console.log

Please remove the console.log
👍 1
eugene marked this conversation as resolved
src/pages/sign/index.tsx Outdated
@ -512,0 +575,4 @@
// setCurrentUserMark(findNextCurrentUserMark(currentUserMarks) || null)
// setIsReadyToSign(isCurrentUserMarksComplete(currentUserMarks))
// }
m commented 2024-08-02 15:39:06 +00:00
Owner
Copy Link

I suggest removing the commented code or providing more details in a comment as to why we should keep it.

I suggest removing the commented code or providing more details in a comment as to why we should keep it.
👍 1
eugene marked this conversation as resolved
src/pages/sign/index.tsx Outdated
@ -922,0 +995,4 @@
// )}
// </Box>
// </>
// )
m commented 2024-08-02 15:39:52 +00:00
Owner
Copy Link

I suggest removing the commented code or providing more details in a comment as to why we should keep it.

I suggest removing the commented code or providing more details in a comment as to why we should keep it.
eugene marked this conversation as resolved
src/pages/verify/index.tsx Outdated
@ -361,1 +381,4 @@
const handleExport = async () => {
// const proverbialUrls = await addMarksV2(Object.values(files)[0].file, meta!)
// await convertToFinalPdf(proverbialUrls)
m commented 2024-08-02 15:40:28 +00:00
Owner
Copy Link

I suggest removing the commented code or providing more details in a comment as to why we should keep it.

I suggest removing the commented code or providing more details in a comment as to why we should keep it.
eugene marked this conversation as resolved
src/types/mark.ts Outdated
@ -3,0 +5,4 @@
// * @key png (pdf page) file hash
// */
// [key: string]: MarkConfigDetails[]
// }
m commented 2024-08-02 15:40:49 +00:00
Owner
Copy Link

I suggest removing the commented code or providing more details in a comment as to why we should keep it.

I suggest removing the commented code or providing more details in a comment as to why we should keep it.
eugene marked this conversation as resolved
eugene added 3 commits 2024-08-05 11:46:48 +00:00
eugene force-pushed issue-99-signing-page-refactor from 37b0ae21e0 to c5e234d792 2024-08-06 09:27:52 +00:00 Compare
eugene added 1 commit 2024-08-06 09:36:03 +00:00
eugene commented 2024-08-08 11:02:29 +00:00
Author
Owner
Copy Link

This PR has been merged as part of the issue-99 PR. Closing.

This PR has been merged as part of the `issue-99` PR. Closing.
eugene closed this pull request 2024-08-08 11:02:29 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
m
Labels
Clear labels   
Kind/Breaking

Breaking change that won't be backward compatible

  
Kind/Bug

Something is not working

  
Kind/Documentation

Documentation changes

  
Kind/Enhancement

Improve existing functionality

  
Kind/Feature

New functionality

  
Kind/Security

This is security issue

  
Kind/Testing

Issue or pull request related to testing

  
Priority
Critical
The priority is critical

  
Priority
High
The priority is high

  
Priority
Low
The priority is low

  
Priority
Medium
The priority is medium

  
Reviewed
Confirmed
Issue has been confirmed

  
Reviewed
Duplicate
This issue or pull request already exists

  
Reviewed
Invalid
Invalid issue

  
Reviewed
Won't Fix
This issue won't be fixed

  
Status
Abandoned
Somebody has started to work on this but abandoned work

  
Status
Blocked
Something is blocking this issue or pull request

  
Status
Need More Info
Feedback is required to reproduce issue or to continue work

No Label
Kind/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sigit/sigit.io#124
No description provided.
Delete Branch "issue-99-signing-page-refactor"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?