-
Notifications
You must be signed in to change notification settings - Fork 31
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
Feat/1804 pdf preview #2710
Feat/1804 pdf preview #2710
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.
I see you did the same as before, I commented on the previous PR and hoped it would be solved differently this time. Anyway, my comments from last time:
- I don't like that you are importing dev-tools components into the application. My expectation is that anything in
features/devtools
can be changed / broken at any time since its only meant to be a tool for developers and don't necessarily live up to the same standards of quality and support as the rest of the app. I would rather you create a separate component for this feature, or alternately moving the existing component outside of dev-tools and importing it there instead so it is more explicit that this cannot just be messed with. - This will just dump any errors from the backend into the modal if PDF-generation fails (as it often does), this is because it is a developer tool, and for a feature I would expect some more user friendly feed-back.
- You are mixing two different concepts here.
setPDFPreview
(old preview usingwindow.print
), andPDFGeneratorPreview
which uses the actual PDF generator in the backend (you can see in dev-tools that there are two different buttons). ThehandleClick
withaction === 'printPreview'
will never be called because you render thePDFGeneratorPreview
instead of theButton
withonClick={handleClick}
.
Just a few comments:
|
Well, its just a matter of priority, we can't prioritize everything. We don't have any tests covering dev-tools, and we don't mind if things break for a little while as it does not affect end users. If developers find some feature essential we should obviously fix it eventually though. |
Quality Gate failedFailed conditions |
Added support for displaying a text if the user is previewing the pdf before the form is submitted
Depends on backend PR: Altinn/app-lib-dotnet#904
Description
Please see acceptance criterias here: #1804
printPreview
action type toActionButton
configRelated Issue(s)
Verification/QA
kind/*
label to this PR for proper release notes grouping