Improve visible signature preview, placement and output workflow - #479
Improve visible signature preview, placement and output workflow#479LadaN62 wants to merge 5 commits into
Conversation
|
Thanks for the PR — I've started reviewing it. The changes span both the signing engines and the JavaFX UI, and the engine-side parts change the generated output, so I want to go through those carefully. About the test you mentioned — I filed it as #480. It passes on a clean When I run into something that needs changing, I may push a commit to your branch instead of sending it back as a review note — kept separate and clearly described, so you can see exactly what changed and object to anything you disagree with. Let me know if you'd prefer to handle those yourself instead. |
|
Hi Josef, Yes, please feel free to push commits directly to my branch whenever you find something that should be changed. I think that will work very well, especially if the changes are kept separate and clearly described as you suggested. That way I can review exactly what was changed and understand the reasoning behind it. “Allow edits by maintainers” is already enabled on the PR, so you should be able to push directly to the branch. And if there is anything you would prefer me to change myself, just let me know. Thanks also for checking the Windows test issue and for opening #480. Ladislav |
|
One question on the output suffix, before I touch anything there. I'd rather not add a second place that decides the output name — there's already That does change your workflow, though: a preference is a poor fit if you switch the marker per document ( |
|
One more note: I fixed the failing Windows tests in the master and then merged to your branch, so the CI is green now. |
|
Hi Josef, Yes, the real purpose of the field is to keep the suffix easy to change, but not necessarily to change it for every document. In my normal workflow, the same suffix is usually used for many consecutive signatures. Ideally, I would like the field to remain visible and editable in the main window, while JSignPdf remembers the last value used and restores it in the next session. In that sense, I think your idea of using "output.suffix" as the single persistent source is a good one. My only preference would be to make that same value editable directly from the signing window, so I don't have to go into Preferences every time I occasionally want to change it. That way there would not be two independent places deciding the output name — it would be the same persistent value, simply exposed in the main UI as well. "txtOutFile" technically covers the use case, but it requires editing the complete file name. The purpose of the suffix field was to let the user change only the suffix quickly and with less chance of making mistakes. Looking further ahead, we're also thinking about signature templates/profiles that could have a default suffix associated with them. In that case, selecting a template could automatically suggest its default suffix, while the visible field would still allow the user to override it manually when needed. I think that would preserve the consistency and persistence of "output.suffix" while keeping the flexibility we were aiming for in everyday use. By the way, the "DL" and "EM" examples left in the code are just remnants from my local workflow/testing; they are not intended to be special or predefined JSignPdf values. The suffix should accept arbitrary text. And thanks for fixing the Windows tests and merging the updated master into my branch. I saw that the CI is green now — much appreciated. Ladislav |
|
One small clarification to my previous comment: by “remember the last value” I meant that only as a fallback when no default suffix has been explicitly configured. If output.suffix is set in Preferences, that should be the startup default. The field in the main window would then allow a temporary override without changing that configured default. |
Summary
This PR improves the visible-signature workflow in the JavaFX UI and aligns the preview more closely with the generated PDF signature.
Changes
${timestamp:<pattern>}.Testing
The changes were tested locally on Windows 11 with Java 21.
mvn clean verifycurrently reaches the DSS test suite but is blocked by an existing upstream test failure:JSignPdfSignatureDrawerTest.textSizeIsCappedOnBothDrawerPathsThe failure is:
U+0053 ('S') is not available in the font Symbol, encoding: SymbolEncodingThe same test and the same error were reproduced separately on a clean checkout of
upstream/masterat commit48d7dd2, without any changes from this PR.