Implement PDF Generation side sheet. - #12619
Merged
aaemnnosttv merged 18 commits intoMay 13, 2026
Merged
Conversation
|
Size Change: +1.4 kB (+0.06%) Total Size: 2.3 MB 📦 View Changed
ℹ️ View Unchanged
|
eugene-manuilov
requested changes
Apr 28, 2026
eugene-manuilov
left a comment
eugene-manuilov
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, @benbowler. Left some feedback for you. Please, take a look.
aaemnnosttv
requested changes
Apr 28, 2026
aaemnnosttv
left a comment
aaemnnosttv
left a comment
Collaborator
There was a problem hiding this comment.
Thanks @benbowler ,
I've left a few additional comments of my own here while being pulled in on the review.
One additional important point which hasn't been raised is that all new files and components should be authored in TypeScript (unless there is a good reason we can't), but everything here has been added in JS.
eugene-manuilov
requested changes
Apr 30, 2026
Collaborator
Author
The key lines in the code you copied are: This PR is a stub that doesn't require the implementation of the datastore. The core forms will be replaced with the correct PDF generation state within #12537 and beyond. |
Collaborator
If #12537 is a dependency for this ticket, we should start with that task first, or not?
#12537 only says to extend the existing widgets infrastructure to support PDF variants. I don't see it mentioning creating a PDF-specific datastore to handle the PDF generation state. |
…pdf-generation-button-and-sidesheet.
eugene-manuilov
requested changes
May 5, 2026
Collaborator
Author
|
@eugene-manuilov I've created the missing datastore ticket (let's prioritise reviewing this and getting it into the next sprint) and addressed the code review comments. |
eugene-manuilov
previously requested changes
May 8, 2026
eugene-manuilov
left a comment
eugene-manuilov
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, @benbowler. There are VRT issues. Could you please take a look? Once fixed, please, assign to @aaemnnosttv for review as well. I have nothing else to add 👍.
…pdf-generation-button-and-sidesheet.
Collaborator
Author
|
Remaining VRT failure and E2E failures are unrelated to this work. |
aaemnnosttv
approved these changes
May 13, 2026
aaemnnosttv
left a comment
aaemnnosttv
left a comment
Collaborator
There was a problem hiding this comment.
Thanks all! I pushed a few extra fixes to get this over the line
DashboardMainApphad a complexity over the limit. There is a lot of room for improvement in this component but I made the smallest change necessary; we can address further improvements in a new issue.- Test failures related to focus trap – seems related to an odd nuance about our tabbable mock not being applied in all instances
- Refactored the use of form state to use the new
useFormValuehook for consistency (also a bit cleaner) + fixed one instance of this which was broken by the update
aaemnnosttv
deleted the
enhancement/12507-pdf-generation-button-and-sidesheet
branch
May 13, 2026 21:50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses issue:
Relevant technical choices
PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist