{"action":"created","comment":{"url":"https://api.github.com/repos/mozilla/pdf.js/pulls/comments/168623064","pull_request_review_id":97023772,"id":168623064,"diff_hunk":"@@ -76,6 +93,36 @@ class PDFViewer extends BaseViewer {\n       location: this._location,\n     });\n   }\n+\n+  _regroupSpreads() {\n+    requestAnimationFrame(() => {","path":"web/pdf_viewer.js","position":57,"original_position":57,"commit_id":"63f6328ad87bb7ffc548abd335ba962b67a6ae5a","original_commit_id":"63f6328ad87bb7ffc548abd335ba962b67a6ae5a","user":{"login":"rhendric","id":1570964,"avatar_url":"https://avatars0.githubusercontent.com/u/1570964?v=4","gravatar_id":"","url":"https://api.github.com/users/rhendric","html_url":"https://github.com/rhendric","followers_url":"https://api.github.com/users/rhendric/followers","following_url":"https://api.github.com/users/rhendric/following{/other_user}","gists_url":"https://api.github.com/users/rhendric/gists{/gist_id}","starred_url":"https://api.github.com/users/rhendric/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rhendric/subscriptions","organizations_url":"https://api.github.com/users/rhendric/orgs","repos_url":"https://api.github.com/users/rhendric/repos","events_url":"https://api.github.com/users/rhendric/events{/privacy}","received_events_url":"https://api.github.com/users/rhendric/received_events","type":"User","site_admin":false},"body":"I didn't want to overengineer this, since the need for `requestAnimationFrame` at all is somewhat speculative. (‘It's a good idea to do heavy DOM manipulation within `rAF` if you care about performance’ is the entirety of my rationale; I confess to not knowing enough about browser rendering internals to examine that maxim in more detail.) That said, the only way that `_regroupSpreads` gets called is if the spread mode is actually changed from its previous value. So calling it multiple times between frames would mean the user is clicking a spread mode button multiple times between frames. Even then, nothing incorrect happens; there's just a redundant manipulation of the DOM. So the upshot is that the worst thing that can happen is that someone can spam the UI with clicks and get a performance hit as a consequence. To that I say... shrug? But implementing the throttling pattern in an abundance of caution wouldn't be hard either. What do you think?","created_at":"2018-02-15T22:06:16Z","updated_at":"2018-02-15T22:06:16Z","html_url":"https://github.com/mozilla/pdf.js/pull/9208#discussion_r168623064","pull_request_url":"https://api.github.com/repos/mozilla/pdf.js/pulls/9208","author_association":"NONE","_links":{"self":{"href":"https://api.github.com/repos/mozilla/pdf.js/pulls/comments/168623064"},"html":{"href":"https://github.com/mozilla/pdf.js/pull/9208#discussion_r168623064"},"pull_request":{"href":"https://api.github.com/repos/mozilla/pdf.js/pulls/9208"}},"in_reply_to_id":168578119},"pull_request":{"url":"https://api.github.com/repos/mozilla/pdf.js/pulls/9208","id":155256636,"html_url":"https://github.com/mozilla/pdf.js/pull/9208","diff_url":"https://github.com/mozilla/pdf.js/pull/9208.diff","patch_url":"https://github.com/mozilla/pdf.js/pull/9208.patch","issue_url":"https://api.github.com/repos/mozilla/pdf.js/issues/9208","number":9208,"state":"open","locked":false,"title":"Add scrolling modes to web viewer","user":{"login":"rhendric","id":1570964,"avatar_url":"https://avatars0.githubusercontent.com/u/1570964?v=4","gravatar_id":"","url":"https://api.github.com/users/rhendric","html_url":"https://github.com/rhendric","followers_url":"https://api.github.com/users/rhendric/followers","following_url":"https://api.github.com/users/rhendric/following{/other_user}","gists_url":"https://api.github.com/users/rhendric/gists{/gist_id}","starred_url":"https://api.github.com/users/rhendric/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rhendric/subscriptions","organizations_url":"https://api.github.com/users/rhendric/orgs","repos_url":"https://api.github.com/users/rhendric/repos","events_url":"https://api.github.com/users/rhendric/events{/privacy}","received_events_url":"https://api.github.com/users/rhendric/received_events","type":"User","site_admin":false},"body":"**Note: these screenshots are out of date, as are the descriptions, somewhat. See [this comment](https://github.com/mozilla/pdf.js/pull/9208#issuecomment-359899094) and [this comment](https://github.com/mozilla/pdf.js/pull/9208#issuecomment-362190512) for the new details.**\r\n\r\nIn addition to the default scrolling mode (vertical), this commit adds horizontal, grid, and grid+cover scrolling, implemented primarily with flexbox CSS.\r\n\r\nYou want some SHOTS???\r\n\r\nThe default: vertical scrolling. Yawn.\r\n![vertical scrolling](https://user-images.githubusercontent.com/1570964/33395115-80211898-d512-11e7-9f9b-901a94bcebd2.png)\r\n\r\nHorizontal scrolling! #2545, that's what's up!\r\n![horizontal scrolling](https://user-images.githubusercontent.com/1570964/33394768-65b0cb9e-d511-11e7-87ce-d6b363dcafbb.png)\r\n\r\nAnd I know my peeps down at #590 have been begging for that GRID SCROLLING!\r\n![grid scrolling, two pages wide](https://user-images.githubusercontent.com/1570964/33394810-88f09d78-d511-11e7-9be8-882631befa53.png)\r\n\r\nAs you can see, weirdly-sized pages are *not a problem* for the mighty flexbox!\r\n\r\nDid you want special cover page handling for your grid scrolling? I GOT YOU.\r\n![grid scrolling with cover page](https://user-images.githubusercontent.com/1570964/33394892-cee8fffa-d511-11e7-93f3-d29a50b80d97.png)\r\n\r\nBut why is it called grid scrolling instead of two-up or dual? Because depending on your zoom and your browser width, it could look like this!\r\n![grid scrolling, three pages wide](https://user-images.githubusercontent.com/1570964/33394931-ec1e56e2-d511-11e7-9326-043ed96525fb.png)\r\n\r\nDon't thank me, thank flexbox!\r\n\r\n---\r\n\r\nI manually tested this in Chrome and Firefox, with large and small PDF files, and made sure it doesn't interfere with presentation mode. I am all about adding those automated tests when submitting pull requests because that's how my momma raised me, but in this case I'm not sure what kind of test to add and/or how! If my reviewer has any ideas, I'm all ears.\r\n\r\n---\r\n\r\nCloses #590, closes #2545, opens the door to your heart.","created_at":"2017-11-29T06:05:52Z","updated_at":"2018-02-15T22:06:16Z","closed_at":null,"merged_at":null,"merge_commit_sha":"bb450285fa52e8f728f297c28f5eccb0a50fcd82","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":164429,"url":"https://api.github.com/repos/mozilla/pdf.js/labels/1-viewer","name":"1-viewer","color":"DDDDDD","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/mozilla/pdf.js/pulls/9208/commits","review_comments_url":"https://api.github.com/repos/mozilla/pdf.js/pulls/9208/comments","review_comment_url":"https://api.github.com/repos/mozilla/pdf.js/pulls/comments{/number}","comments_url":"https://api.github.com/repos/mozilla/pdf.js/issues/9208/comments","statuses_url":"https://api.github.com/repos/mozilla/pdf.js/statuses/63f6328ad87bb7ffc548abd335ba962b67a6ae5a","head":{"label":"rhendric:scrolling-modes","ref":"scrolling-modes","sha":"63f6328ad87bb7ffc548abd335ba962b67a6ae5a","user":{"login":"rhendric","id":1570964,"avatar_url":"https://avatars0.githubusercontent.com/u/1570964?v=4","gravatar_id":"","url":"https://api.github.com/users/rhendric","html_url":"https://github.com/rhendric","followers_url":"https://api.github.com/users/rhendric/followers","following_url":"https://api.github.com/users/rhendric/following{/other_user}","gists_url":"https://api.github.com/users/rhendric/gists{/gist_id}","starred_url":"https://api.github.com/users/rhendric/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rhendric/subscriptions","organizations_url":"https://api.github.com/users/rhendric/orgs","repos_url":"https://api.github.com/users/rhendric/repos","events_url":"https://api.github.com/users/rhendric/events{/privacy}","received_events_url":"https://api.github.com/users/rhendric/received_events","type":"User","site_admin":false},"repo":{"id":112430766,"name":"pdf.js","full_name":"rhendric/pdf.js","owner":{"login":"rhendric","id":1570964,"avatar_url":"https://avatars0.githubusercontent.com/u/1570964?v=4","gravatar_id":"","url":"https://api.github.com/users/rhendric","html_url":"https://github.com/rhendric","followers_url":"https://api.github.com/users/rhendric/followers","following_url":"https://api.github.com/users/rhendric/following{/other_user}","gists_url":"https://api.github.com/users/rhendric/gists{/gist_id}","starred_url":"https://api.github.com/users/rhendric/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rhendric/subscriptions","organizations_url":"https://api.github.com/users/rhendric/orgs","repos_url":"https://api.github.com/users/rhendric/repos","events_url":"https://api.github.com/users/rhendric/events{/privacy}","received_events_url":"https://api.github.com/users/rhendric/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/rhendric/pdf.js","description":"PDF Reader in JavaScript","fork":true,"url":"https://api.github.com/repos/rhendric/pdf.js","forks_url":"https://api.github.com/repos/rhendric/pdf.js/forks","keys_url":"https://api.github.com/repos/rhendric/pdf.js/keys{/key_id}","collaborators_url":"https://api.github.com/repos/rhendric/pdf.js/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/rhendric/pdf.js/teams","hooks_url":"https://api.github.com/repos/rhendric/pdf.js/hooks","issue_events_url":"https://api.github.com/repos/rhendric/pdf.js/issues/events{/number}","events_url":"https://api.github.com/repos/rhendric/pdf.js/events","assignees_url":"https://api.github.com/repos/rhendric/pdf.js/assignees{/user}","branches_url":"https://api.github.com/repos/rhendric/pdf.js/branches{/branch}","tags_url":"https://api.github.com/repos/rhendric/pdf.js/tags","blobs_url":"https://api.github.com/repos/rhendric/pdf.js/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/rhendric/pdf.js/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/rhendric/pdf.js/git/refs{/sha}","trees_url":"https://api.github.com/repos/rhendric/pdf.js/git/trees{/sha}","statuses_url":"https://api.github.com/repos/rhendric/pdf.js/statuses/{sha}","languages_url":"https://api.github.com/repos/rhendric/pdf.js/languages","stargazers_url":"https://api.github.com/repos/rhendric/pdf.js/stargazers","contributors_url":"https://api.github.com/repos/rhendric/pdf.js/contributors","subscribers_url":"https://api.github.com/repos/rhendric/pdf.js/subscribers","subscription_url":"https://api.github.com/repos/rhendric/pdf.js/subscription","commits_url":"https://api.github.com/repos/rhendric/pdf.js/commits{/sha}","git_commits_url":"https://api.github.com/repos/rhendric/pdf.js/git/commits{/sha}","comments_url":"https://api.github.com/repos/rhendric/pdf.js/comments{/number}","issue_comment_url":"https://api.github.com/repos/rhendric/pdf.js/issues/comments{/number}","contents_url":"https://api.github.com/repos/rhendric/pdf.js/contents/{+path}","compare_url":"https://api.github.com/repos/rhendric/pdf.js/compare/{base}...{head}","merges_url":"https://api.github.com/repos/rhendric/pdf.js/merges","archive_url":"https://api.github.com/repos/rhendric/pdf.js/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/rhendric/pdf.js/downloads","issues_url":"https://api.github.com/repos/rhendric/pdf.js/issues{/number}","pulls_url":"https://api.github.com/repos/rhendric/pdf.js/pulls{/number}","milestones_url":"https://api.github.com/repos/rhendric/pdf.js/milestones{/number}","notifications_url":"https://api.github.com/repos/rhendric/pdf.js/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/rhendric/pdf.js/labels{/name}","releases_url":"https://api.github.com/repos/rhendric/pdf.js/releases{/id}","deployments_url":"https://api.github.com/repos/rhendric/pdf.js/deployments","created_at":"2017-11-29T05:26:21Z","updated_at":"2017-11-29T05:26:25Z","pushed_at":"2018-02-04T16:01:07Z","git_url":"git://github.com/rhendric/pdf.js.git","ssh_url":"git@github.com:rhendric/pdf.js.git","clone_url":"https://github.com/rhendric/pdf.js.git","svn_url":"https://github.com/rhendric/pdf.js","homepage":"","size":73345,"stargazers_count":0,"watchers_count":0,"language":"JavaScript","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":null,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"mozilla:master","ref":"master","sha":"db9f71fe6bad91d6b8426abe63bb9054059560e1","user":{"login":"mozilla","id":131524,"avatar_url":"https://avatars2.githubusercontent.com/u/131524?v=4","gravatar_id":"","url":"https://api.github.com/users/mozilla","html_url":"https://github.com/mozilla","followers_url":"https://api.github.com/users/mozilla/followers","following_url":"https://api.github.com/users/mozilla/following{/other_user}","gists_url":"https://api.github.com/users/mozilla/gists{/gist_id}","starred_url":"https://api.github.com/users/mozilla/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mozilla/subscriptions","organizations_url":"https://api.github.com/users/mozilla/orgs","repos_url":"https://api.github.com/users/mozilla/repos","events_url":"https://api.github.com/users/mozilla/events{/privacy}","received_events_url":"https://api.github.com/users/mozilla/received_events","type":"Organization","site_admin":false},"repo":{"id":1663468,"name":"pdf.js","full_name":"mozilla/pdf.js","owner":{"login":"mozilla","id":131524,"avatar_url":"https://avatars2.githubusercontent.com/u/131524?v=4","gravatar_id":"","url":"https://api.github.com/users/mozilla","html_url":"https://github.com/mozilla","followers_url":"https://api.github.com/users/mozilla/followers","following_url":"https://api.github.com/users/mozilla/following{/other_user}","gists_url":"https://api.github.com/users/mozilla/gists{/gist_id}","starred_url":"https://api.github.com/users/mozilla/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mozilla/subscriptions","organizations_url":"https://api.github.com/users/mozilla/orgs","repos_url":"https://api.github.com/users/mozilla/repos","events_url":"https://api.github.com/users/mozilla/events{/privacy}","received_events_url":"https://api.github.com/users/mozilla/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mozilla/pdf.js","description":"PDF Reader in JavaScript","fork":false,"url":"https://api.github.com/repos/mozilla/pdf.js","forks_url":"https://api.github.com/repos/mozilla/pdf.js/forks","keys_url":"https://api.github.com/repos/mozilla/pdf.js/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mozilla/pdf.js/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mozilla/pdf.js/teams","hooks_url":"https://api.github.com/repos/mozilla/pdf.js/hooks","issue_events_url":"https://api.github.com/repos/mozilla/pdf.js/issues/events{/number}","events_url":"https://api.github.com/repos/mozilla/pdf.js/events","assignees_url":"https://api.github.com/repos/mozilla/pdf.js/assignees{/user}","branches_url":"https://api.github.com/repos/mozilla/pdf.js/branches{/branch}","tags_url":"https://api.github.com/repos/mozilla/pdf.js/tags","blobs_url":"https://api.github.com/repos/mozilla/pdf.js/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mozilla/pdf.js/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mozilla/pdf.js/git/refs{/sha}","trees_url":"https://api.github.com/repos/mozilla/pdf.js/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mozilla/pdf.js/statuses/{sha}","languages_url":"https://api.github.com/repos/mozilla/pdf.js/languages","stargazers_url":"https://api.github.com/repos/mozilla/pdf.js/stargazers","contributors_url":"https://api.github.com/repos/mozilla/pdf.js/contributors","subscribers_url":"https://api.github.com/repos/mozilla/pdf.js/subscribers","subscription_url":"https://api.github.com/repos/mozilla/pdf.js/subscription","commits_url":"https://api.github.com/repos/mozilla/pdf.js/commits{/sha}","git_commits_url":"https://api.github.com/repos/mozilla/pdf.js/git/commits{/sha}","comments_url":"https://api.github.com/repos/mozilla/pdf.js/comments{/number}","issue_comment_url":"https://api.github.com/repos/mozilla/pdf.js/issues/comments{/number}","contents_url":"https://api.github.com/repos/mozilla/pdf.js/contents/{+path}","compare_url":"https://api.github.com/repos/mozilla/pdf.js/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mozilla/pdf.js/merges","archive_url":"https://api.github.com/repos/mozilla/pdf.js/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mozilla/pdf.js/downloads","issues_url":"https://api.github.com/repos/mozilla/pdf.js/issues{/number}","pulls_url":"https://api.github.com/repos/mozilla/pdf.js/pulls{/number}","milestones_url":"https://api.github.com/repos/mozilla/pdf.js/milestones{/number}","notifications_url":"https://api.github.com/repos/mozilla/pdf.js/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mozilla/pdf.js/labels{/name}","releases_url":"https://api.github.com/repos/mozilla/pdf.js/releases{/id}","deployments_url":"https://api.github.com/repos/mozilla/pdf.js/deployments","created_at":"2011-04-26T06:32:03Z","updated_at":"2018-02-15T20:45:15Z","pushed_at":"2018-02-15T12:23:29Z","git_url":"git://github.com/mozilla/pdf.js.git","ssh_url":"git@github.com:mozilla/pdf.js.git","clone_url":"https://github.com/mozilla/pdf.js.git","svn_url":"https://github.com/mozilla/pdf.js","homepage":"","size":73804,"stargazers_count":20987,"watchers_count":20987,"language":"JavaScript","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":5143,"mirror_url":null,"archived":false,"open_issues_count":686,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0"},"forks":5143,"open_issues":686,"watchers":20987,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/mozilla/pdf.js/pulls/9208"},"html":{"href":"https://github.com/mozilla/pdf.js/pull/9208"},"issue":{"href":"https://api.github.com/repos/mozilla/pdf.js/issues/9208"},"comments":{"href":"https://api.github.com/repos/mozilla/pdf.js/issues/9208/comments"},"review_comments":{"href":"https://api.github.com/repos/mozilla/pdf.js/pulls/9208/comments"},"review_comment":{"href":"https://api.github.com/repos/mozilla/pdf.js/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/mozilla/pdf.js/pulls/9208/commits"},"statuses":{"href":"https://api.github.com/repos/mozilla/pdf.js/statuses/63f6328ad87bb7ffc548abd335ba962b67a6ae5a"}},"author_association":"NONE"}}
{"actor":{"display_login":"rhendric"}}