Skip to content

Convert GrabToPan to a standard class #14255

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

Merged
merged 2 commits into from
Nov 13, 2021

Conversation

Snuffleupagus
Copy link
Collaborator

  • [web/grab_to_pan.js] Inline the isLeftMouseReleased helper function

    Given the support information listed in the function itself, the MDN compatibility data, and the currently supported browsers in the PDF.js project we should be able to simplify the code by inlining the function instead.

  • Convert GrabToPan to a standard class

    This code is the last piece[1] of the viewer that's not using standard classes, and by converting this code we get rid of some now unneeded boilerplate code (slightly reducing the size of the built web/viewer.js file).
    Note that while this code was originally imported from a separate repository, it was last sync-ed with upstream five years ago which is why this re-factoring should be OK as far as I'm concerned (and we've done some other clean-up since then as well).


    [1] Technically the web/debugger.js file is left as well, however that code is first of all not bundled in the built web/viewer.js file and secondly it's not even loaded by default either.

@Snuffleupagus Snuffleupagus changed the title Grab to pan class Convert GrabToPan to a standard class Nov 9, 2021
Copy link
Contributor

@timvandermeij timvandermeij left a comment

Choose a reason for hiding this comment

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

r=me with these two comments addressed; thanks!

Given the support information listed in the function itself, the [MDN compatibility data](https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/buttons#browser_compatibility), and the [currently supported browsers](https://github.com/mozilla/pdf.js/blob/4bb9de4b008dca2cdccf770bc359ae21971debb0/gulpfile.js#L79-L87) in the PDF.js project we should be able to simplify the code by inlining the function instead.
This code is the last piece[1] of the viewer that's not using standard `class`es, and by converting this code we get rid of some now unneeded boilerplate code (slightly reducing the size of the *built* `web/viewer.js` file).
Note that while this code was originally imported from a separate repository, it was last sync-ed with upstream *five years* ago which is why this re-factoring should be OK as far as I'm concerned (and we've done some other clean-up since then as well).

---
[1] Technically the `web/debugger.js` file is left as well, however that code is first of all not bundled in the *built* `web/viewer.js` file and secondly it's not even loaded by default either.
@mozilla mozilla deleted a comment from pdfjsbot Nov 13, 2021
@mozilla mozilla deleted a comment from pdfjsbot Nov 13, 2021
@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/3c361148c796d8c/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/3c361148c796d8c/output.txt

Total script time: 4.49 mins

Published

@Snuffleupagus Snuffleupagus merged commit 712621b into mozilla:master Nov 13, 2021
@Snuffleupagus Snuffleupagus deleted the GrabToPan-class branch November 13, 2021 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants