-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
[editor] Add support for saving a newly added FreeText #14978
Conversation
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/8b7e7a4cf10a64d/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/c8b74220bff33c7/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/8b7e7a4cf10a64d/output.txt Total script time: 26.58 mins
Image differences available at: http://54.241.84.105:8877/8b7e7a4cf10a64d/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/c8b74220bff33c7/output.txt Total script time: 29.30 mins
Image differences available at: http://54.193.163.58:8877/c8b74220bff33c7/reftest-analyzer.html#web=eq.log |
src/display/editor/editor.js
Outdated
* Get the translation to use to position this editor when | ||
* it's created. |
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.
* Get the translation to use to position this editor when | |
* it's created. | |
* Get the translation used to position this editor when it's created. |
src/display/editor/freetext.js
Outdated
document.documentElement.style.setProperty( | ||
"--freetext-padding", | ||
`${INTERNAL_PADDING}px` | ||
); |
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.
How about simply reading the padding value as defined in the CSS file, rather than overwriting it here (with a value it should already have)?
Something like this could perhaps work:
const style = getComputedStyle(document.documentElement);
this._internalPadding = parseInt(
style.getPropertyValue("--freetext-padding"),
10
);
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.
r=me, with one more comment and all tests passing; thank you!
src/display/editor/freetext.js
Outdated
document.documentElement.style.setProperty( | ||
"--freetext-line-height", | ||
LINE_FACTOR | ||
); |
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.
Unless I'm completely misunderstanding the purpose of this code, you basically want to ensure that the CSS variable (as defined in web/annotation_editor_layer_builder.css
) won't accidentally diverge from the internal constant?
If so, how about we just assert that in non-PRODUCTION mode instead?
if (
typeof PDFJSDev === "undefined" ||
PDFJSDev.test("!PRODUCTION || TESTING")
) {
const lineHeight = parseFloat(
style.getPropertyValue("--freetext-line-height")
);
assert(
lineHeight === LINE_FACTOR,
"Update the CSS variable to agree with the constant."
);
}
if (!AnnotationEditorLayer._l10nInitialized) { | ||
AnnotationEditorLayer._l10nInitialized = true; |
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.
Also, maybe just call the property _initialized
now since this is no longer just for l10n-handling?
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/53f6e5884b980be/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/f0dbdb42372f9eb/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/53f6e5884b980be/output.txt Total script time: 26.72 mins
Image differences available at: http://54.241.84.105:8877/53f6e5884b980be/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/f0dbdb42372f9eb/output.txt Total script time: 28.45 mins
|
No description provided.