Skip to content
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

Fixed text annotation comb input box #13690

Merged
merged 1 commit into from
Jul 9, 2021

Conversation

symtalha14
Copy link
Contributor

I have tried to address issue #12865. Please look at it and suggest changes if any.

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't include unrelated files in your patches, hence the package-lock.json must be removed.

Furthermore, have you successfully tested this manually in the viewer and do all of the tests pass with this patch?
Note that these changes are from PR #7649, and see also https://github.com/mozilla/pdf.js/wiki/Contributing#4-run-lint-and-testing

Finally, please provide a meaningful commit-message (and PR description) that actually describes the changes; note that "Solves issue #12865" isn't really helpful/useful in general.

web/annotation_layer_builder.css Outdated Show resolved Hide resolved
@symtalha14 symtalha14 changed the title Solves issue #12865 Text annotation comb input box fixed Jul 7, 2021
@symtalha14
Copy link
Contributor Author

symtalha14 commented Jul 7, 2021

So basically a CSS focus event style was modifying the input box width whenever the input box got focus. It modified the width from 100% to 115%. According to a comment in there, it was done to adjust the letter spacing of the characters in that input box. I have reduced the expanded width from 115% to 103% and it works fine now and passes all the unit tests/reference image tests successfully.

@symtalha14 symtalha14 force-pushed the text-annotation-comb branch 2 times, most recently from 91028a0 to d7daca2 Compare July 7, 2021 17:44
@symtalha14 symtalha14 changed the title Text annotation comb input box fixed Fixed text annotation comb input box Jul 7, 2021
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented Jul 7, 2021

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 1

Live output at: http://54.67.70.0:8877/6a169706eb2f08e/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Jul 7, 2021

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/6a169706eb2f08e/output.txt

Total script time: 4.96 mins

Published

@timvandermeij
Copy link
Contributor

I did a quick check with my original PDF file and so far it looks like this works. Note that this is dynamic behavior that the tests can't cover, so because we have to rely on manual testing here I'd like to take some more time to play with more PDFs later to make sure it works in all scenarios.

@symtalha14
Copy link
Contributor Author

symtalha14 commented Jul 7, 2021

I did a quick check with my original PDF file and so far it looks like this works. Note that this is dynamic behavior that the tests can't cover, so because we have to rely on manual testing here I'd like to take some more time to play with more PDFs later to make sure it works in all scenarios.

I get your point. I have checked in some sample PDFs from my side as well, seems fine. Let me know if you find any malfunction.

@symtalha14
Copy link
Contributor Author

symtalha14 commented Jul 8, 2021

Hey, any updates? @timvandermeij

@timvandermeij timvandermeij merged commit a6d2e78 into mozilla:master Jul 9, 2021
@timvandermeij
Copy link
Contributor

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants