-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Text widget annotations: implement maximum length and text alignment #7622
Text widget annotations: implement maximum length and text alignment #7622
Conversation
0a3d3df
to
334c6ac
Compare
This seems fine, thank you! r=me, with passing tests and with the Reviewed 1 of 1 files at r1. src/display/annotation_layer.js, line 461 at r1 (raw file):
Just something I noticed in passing: This could perhaps be something worth to look into in a future PR, since there's also a src/display/annotation_layer.js, line 468 at r1 (raw file):
Would it make sense to make this a constant instead, e.g. Comments from Reviewable |
Moreover, we refactor the code a bit to extract code that is shared between the two branches and we only apply text alignment (and create the array) when it is actually defined, since it's optional and left is already the default.
334c6ac
to
be485f5
Compare
The comment has been addressed and I followed the specification better by making sure that the values we use for maximum length and text alignment are integers (as other values would not work). This is trivial and there are probably no such broken PDFs, but let's be safe nevertheless. In the meta issue, I added a note about the (probably dead) font code and about sanitizing the values for maximum length/text alignment in the core layer (so that this display layer code is only getting either |
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/e7ebcdf33c06588/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/e7ebcdf33c06588/output.txt Total script time: 1.27 mins Published |
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.22.172.223:8877/8b2a55d4d07816b/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/b4d49b1ff47306a/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/8b2a55d4d07816b/output.txt Total script time: 24.23 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/b4d49b1ff47306a/output.txt Total script time: 39.34 mins
|
Moreover, we refactor the code a bit to extract code that is shared between the two branches and we only apply text alignment (and create the array) when it is actually defined, since it's optional and left is already the default.
@Snuffleupagus Could you review this PR? This patch is a part of #7613 and improves the support for text widget annotations. You can test this by opening the
f1040.pdf
file and noticing that you can now only enter two characters into the top right input box (with 20 before it) and that most fields in the "Income" section and now right-aligned.