[MM-29881] Add ability to zoom in/out on PDFs #5361

shred86 · 2020-04-21T04:10:29Z

Summary

Adds the ability to zoom in and out on PDFs. This required a change in how ViewImageModal is initially rendered. Instead of displaying based on the size of the PDF (or other supported media), it now opens to height: 90vh and width: 90:vw. This was to prevent the modal from continuously expanding and shrinking when the zoom in/out functionality was used. Additional notes:

Implemented most of the "core" zoom controls and methods to include a new scale state in ViewImageModal so it can be applied to other types of media if desired.
The zoom in/out is occurring by adjusting the scale factor in getViewport from PDF js. The reason I took this approach was to provide a higher quality PDF when you zoom in. There's some PDFs that if you try to zoom using the initial render size/scale factor, it's very blurry and hard to read. The other implementation option is to just render the PDF to a higher quality initially, then use pure CSS for the zoom in/out functionality. This would make it easier to implement with other media types (I think).
Refactored some of the code to display the buttons for consistency and to make it a bit cleaner.
Icons were made by me. I'm not a graphic designer by any means, so I'm open to change it to anything.

One minor "issue" where the horizontal scroll bar doesn't show up unless you scroll all the way to the bottom. I've spent the afternoon trying to figure this one out but will continue to investigate. I'm assuming it's just a styling issue.

Ticket Link

https://mattermost.atlassian.net/browse/MM-29881

Related Pull Requests

N/A

Screenshots

Video Clip: https://imgur.com/3JieFTx

components/view_image/view_image.jsx

mattermod · 2020-04-21T07:12:46Z

Test server destroyed

mattermod · 2020-04-21T08:00:43Z

Creating a new SpinWick test server using Mattermost Cloud.

mattermod · 2020-04-21T08:07:29Z

Mattermost test server created! 🎉

Access here: https://mattermost-webapp-pr-5361.test.mattermost.cloud

Account Type	Username	Password
Admin	sysadmin	Sys@dmin123
User	user-1	User-1@123

esethna · 2020-04-21T15:34:45Z

This is sweet! Adding @andrewbrown00 to do a UX review

mattermod · 2020-04-22T01:00:19Z

New commit detected. SpinWick will upgrade if the updated docker image is available.

mattermod · 2020-04-22T01:16:28Z

Mattermost test server updated with git commit 2adf5c66cbb15b97cb3b1a3fd1e68b13d8631bc4.

Access here: https://mattermost-webapp-pr-5361.test.mattermost.cloud

shred86 · 2020-04-22T02:45:29Z

Noticing some errors in the console when you try to click the zoom in or out button very quickly. Occurs more frequently with more complex PDFs:

pdf.js:9770 Uncaught (in promise) Error: Cannot use the same canvas during multiple render() operations. Use different canvas or ensure previous operations were cancelled or completed.

I'm guessing this is because the previous render hasn't finished yet when a new render is called?

shred86 · 2020-04-22T04:47:56Z

components/view_image/view_image.jsx

+                <div>
+                    <div
+                        className='modal-zoom-in'
+                        onClick={this.handleZoomIn}


Perhaps a possible solution is using debounce from lodash to set a min time between clicks to call the function as well as a max wait time to call. Something like:

onClick={debounce(this.handleZoomIn, 300, {maxWait: 300})}

However, the errors I mentioned in my comment still exists (at a lower rate) for larger file sized PDFs, so I'm not sure if this is the best solution.

andrewbrown00

Hi @shred86 thanks for the awesome contribution 🎉

I completed the UX review and added my feedback and recommended improvements below.

Items to resolve

Scrolling no longer shows any additional pages, I only see the first page. This is a regression and should align with the existing behavior.
Don't maintain the last zoomed size when closing, always reset the preview size back to the default when closing and reopening the preview modal.
Make sure the Get a public link logic still works, the link is missing in this PR.

Current	This PR

Recommend improvements

Let's change the style of the + and - icons to use the mattermost icons font. This should be available in the repo. If not, let me know and I'll get you what you need.
Let's add a Fit to width and Reset zoom toggle button in between the icon buttons. Check out Gmail as a great reference for the expected behavior when toggling between the states.
The Zoom out button should be disabled and dimmed when a user clicks to open the preview modal. Let's avoid letting users zooming below the default preview size. Also, clicking zoom out should stop zooming after the preview reaches the default (size when first opened) size.
The Zoom in button should be disabled and dimmed when a user hits a max size based on allowing the user to click the + button 6 times; we may need to adjust this + or - a few clicks.
Let's update the style of the Download to read more pages button to be more prominent and align with our new style updates.

Prototype

I've created a fully interactive prototype to show the behavior and styles when clicking the zoom icons.

Note, tapping the space bar on the first screen will (fake) scroll to last page. From the last screen, just click on the document to reset the prototype back to the first page.

Also, as I previously mentioned, the Gmail preview modal is a great reference for the expected functional behavior/timing when zooming in and out.

shred86 · 2020-04-25T20:20:08Z

Thanks for the feedback!

Items to resolve

Fixed.
This should be easy to resolve...maybe.
Public links was just turned off in that Mattermost instance by default. I just turned it on.

Recommend improvements

I see the fonts directory but the only two icons I'm seeing in mattermosticons.ttf is a pin and clock looking icon. Where can I find the +, - and magnifying glass icons you have in your prototype?
Reset zoom will be easy. Fit to zoom, I'll have to mess around with this.
I'll see what I can do about the disable/dimming at max zoom out. I think users should be able to zoom out past the default scale. The reason being is PDFs render to a scale of 1 (this is the current behavior). If the PDF resolution is larger than the modal, then you will want to zoom out in some cases. Now, some logic could be put in place to maybe check to see if the PDF resolution is larger than the modal view to set a different default scale value, but that may have to be a separate PR.
Same as above. Styling stuff should be do-able.
I should be able to do that.

This will likely take me some time but I'll get as far as I can. I'm hoping someone will have some tips on how to resolve this issue.

esethna · 2020-04-30T22:25:37Z

Hi @shred86 let us know if you need some help addressing the items identified

esethna · 2020-04-30T22:26:35Z

I see the fonts directory but the only two icons I'm seeing in mattermosticons.ttf is a pin and clock looking icon. Where can I find the +, - and magnifying glass icons you have in your prototype?

@andrewbrown00 can you help answer this?

andrewbrown00 · 2020-05-01T22:30:15Z

Hi @shred86, are you able to see the fonts in this directory? You can copy and paste the glyph or check the show codes option to get the hex values. It should look like this:

Glyphs	Codes

I see the fonts directory but the only two icons I'm seeing in mattermosticons.ttf is a pin and clock looking icon. Where can I find the +, - and magnifying glass icons you have in your prototype?

shred86 · 2020-05-02T15:57:13Z

@andrewbrown00 I'm not sure which directory you're referring to. I'm looking at the fonts/mattermosticons on this repository. Can you provide me a link to the directory you have pictured of the mattermost icons?

Also, do I need to create a new jsx file for these icons individually in the components/widgets/icons directory or can I just use this mattermosticon font type directly? Unfortunately styling is a weak area for me so I may need some assistance on this one.

shred86 · 2020-05-02T17:07:59Z

Just made a commit with where I'm at. I've made most of the changes requested from the UX review but here's what's left:

Need to update icons using mattermosticons (currently just using regular text as a place holder).
Determine a solution for the issue I mentioned above. Using debounce helps, but with very large PDFs, it still occurs.
Update Download to read more pages button.
Add a "Fit to width" option.

I should be able to take care of the first one if I can figure out how to access the mattermost icons. The second option I'll need some feedback/assistance. I'd recommend the last two we save for another PR unless someone else wants to work on it.

andrewbrown00 · 2020-05-05T15:39:31Z

@asaadmahmood will you help @shred86 to make sure we are using the latest version of the mattermosticon font.

@andrewbrown00 I'm not sure which directory you're referring to. I'm looking at the fonts/mattermosticons on this repository. Can you provide me a link to the directory you have pictured of the mattermost icons?

Also, do I need to create a new jsx file for these icons individually in the components/widgets/icons directory or can I just use this mattermosticon font type directly? Unfortunately styling is a weak area for me so I may need some assistance on this one.

asaadmahmood · 2020-05-05T17:31:49Z

@shred86 The icons are already there, you can find all the icons this directory.

There you would be able to see the class names of all the icons.
You can use each icon like this:
<i className='icon icon-arrow-right'/>

shred86 · 2020-05-08T03:59:21Z

Appreciate the help! Some remaining issues:

~~There doesn't appear to be a icon-minus, so I'm just using icon-minus-box as a placeholder (see image below).~~
I'm not sure how to implement the "fit to zoom" option. I propose we just have a "reset zoom" option in the middle for now and let someone else make another PR to add this feature if it's still desired.
Still not sure what is to be done about the issue I mentioned above.

asaadmahmood · 2020-10-19T10:06:46Z

@shred86 Pushed and fixed.

mm-cloud-bot · 2020-10-19T12:41:26Z

Creating a new SpinWick test server using Mattermost Cloud.

mm-cloud-bot · 2020-10-19T12:43:04Z

Mattermost test server created! 🎉

Access here: https://mattermost-webapp-pr-5361.test.mattermost.cloud

Account Type	Username	Password
Admin	sysadmin	Sys@dmin123
User	user-1	User-1@123

jgilliam17 · 2020-10-20T19:29:18Z

Thank you @shred86
Tested, looks good to merge.
-Verified zoom in, out and reset for PDF files. Verified get public link and download buttons. Zoom options are not available when previewing files that do not support zooming. Scroll bars visible in both, light and dark themes.

mm-cloud-bot · 2020-10-20T19:29:53Z

Test server destroyed

jgilliam17 · 2020-10-20T19:47:18Z

@amyblais I'll create a Jira Ticket and link here shortly.
https://mattermost.atlassian.net/browse/MM-29881

hanzei · 2020-10-21T12:32:56Z

Thanks you very much for this amazing contribution @shred86 🚀 🎉

Co-authored-by: Asaad Mahmood <[email protected]> Co-authored-by: Ben Schumacher <[email protected]>

shred86 commented Apr 21, 2020

View reviewed changes

components/view_image/view_image.jsx Outdated Show resolved Hide resolved

hanzei requested a review from esethna April 21, 2020 06:52

hanzei added 1: PM Review Requires review by a product manager 1: UX Review Requires review by a UX Designer 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Apr 21, 2020

mattermod removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Apr 21, 2020

hanzei added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Apr 21, 2020

esethna requested a review from andrewbrown00 April 21, 2020 15:40

shred86 commented Apr 22, 2020

View reviewed changes

andrewbrown00 suggested changes Apr 24, 2020

View reviewed changes

cpanato removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Apr 28, 2020

andrewbrown00 self-requested a review May 5, 2020 15:40

asaadmahmood added 3 commits October 19, 2020 15:05

Updating scrollbar for pdf zoom

c6e9e98

Updating pdf preview css

1a4ceca

Updating scrollbar on dark theme for pdfs

f5b70f1

asaadmahmood force-pushed the pdf_zoom branch from e595dc9 to f5b70f1 Compare October 19, 2020 10:06

asaadmahmood approved these changes Oct 19, 2020

View reviewed changes

jgilliam17 added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Oct 19, 2020

amyblais removed this from the v5.29.0 milestone Oct 20, 2020

jgilliam17 approved these changes Oct 20, 2020

View reviewed changes

jgilliam17 added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Oct 20, 2020

jgilliam17 changed the title ~~Add ability to zoom in/out on PDFs~~ [MM-29881]Add ability to zoom in/out on PDFs Oct 20, 2020

hanzei added this to the v5.30.0 milestone Oct 21, 2020

hanzei changed the title ~~[MM-29881]Add ability to zoom in/out on PDFs~~ [MM-29881] Add ability to zoom in/out on PDFs Oct 21, 2020

hanzei added 3 commits October 21, 2020 13:55

Merge remote-tracking branch 'origin/master' into pdf_zoom

81d42d6

Fix i18n

2d360ca

Update snapshots

8906c2d

hanzei merged commit 22d3424 into mattermost:master Oct 21, 2020

jfrerich pushed a commit that referenced this pull request Oct 23, 2020

[MM-29881] Add ability to zoom in/out on PDFs (#5361)

078db1f

Co-authored-by: Asaad Mahmood <[email protected]> Co-authored-by: Ben Schumacher <[email protected]>

calebroseland pushed a commit that referenced this pull request Oct 27, 2020

[MM-29881] Add ability to zoom in/out on PDFs (#5361)

0d999f6

Co-authored-by: Asaad Mahmood <[email protected]> Co-authored-by: Ben Schumacher <[email protected]>

shred86 deleted the pdf_zoom branch November 9, 2020 01:31

amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Nov 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MM-29881] Add ability to zoom in/out on PDFs #5361

[MM-29881] Add ability to zoom in/out on PDFs #5361

shred86 commented Apr 21, 2020 •

edited by jgilliam17

Loading

mattermod commented Apr 21, 2020

mattermod commented Apr 21, 2020

mattermod commented Apr 21, 2020

esethna commented Apr 21, 2020

mattermod commented Apr 22, 2020

mattermod commented Apr 22, 2020

shred86 commented Apr 22, 2020 •

edited

Loading

shred86 Apr 22, 2020 •

edited

Loading

andrewbrown00 left a comment •

edited

Loading

shred86 commented Apr 25, 2020 •

edited

Loading

esethna commented Apr 30, 2020

esethna commented Apr 30, 2020

andrewbrown00 commented May 1, 2020

shred86 commented May 2, 2020 •

edited

Loading

shred86 commented May 2, 2020

andrewbrown00 commented May 5, 2020

asaadmahmood commented May 5, 2020

shred86 commented May 8, 2020 •

edited

Loading

asaadmahmood commented Oct 19, 2020

mm-cloud-bot commented Oct 19, 2020

mm-cloud-bot commented Oct 19, 2020

jgilliam17 commented Oct 20, 2020

mm-cloud-bot commented Oct 20, 2020

jgilliam17 commented Oct 20, 2020 •

edited

Loading

hanzei commented Oct 21, 2020

[MM-29881] Add ability to zoom in/out on PDFs #5361

[MM-29881] Add ability to zoom in/out on PDFs #5361

Conversation

shred86 commented Apr 21, 2020 • edited by jgilliam17 Loading

Summary

Ticket Link

Related Pull Requests

Screenshots

mattermod commented Apr 21, 2020

mattermod commented Apr 21, 2020

mattermod commented Apr 21, 2020

esethna commented Apr 21, 2020

mattermod commented Apr 22, 2020

mattermod commented Apr 22, 2020

shred86 commented Apr 22, 2020 • edited Loading

shred86 Apr 22, 2020 • edited Loading

Choose a reason for hiding this comment

andrewbrown00 left a comment • edited Loading

Choose a reason for hiding this comment

shred86 commented Apr 25, 2020 • edited Loading

esethna commented Apr 30, 2020

esethna commented Apr 30, 2020

andrewbrown00 commented May 1, 2020

shred86 commented May 2, 2020 • edited Loading

shred86 commented May 2, 2020

andrewbrown00 commented May 5, 2020

asaadmahmood commented May 5, 2020

shred86 commented May 8, 2020 • edited Loading

asaadmahmood commented Oct 19, 2020

mm-cloud-bot commented Oct 19, 2020

mm-cloud-bot commented Oct 19, 2020

jgilliam17 commented Oct 20, 2020

mm-cloud-bot commented Oct 20, 2020

jgilliam17 commented Oct 20, 2020 • edited Loading

hanzei commented Oct 21, 2020

shred86 commented Apr 21, 2020 •

edited by jgilliam17

Loading

shred86 commented Apr 22, 2020 •

edited

Loading

shred86 Apr 22, 2020 •

edited

Loading

andrewbrown00 left a comment •

edited

Loading

shred86 commented Apr 25, 2020 •

edited

Loading

shred86 commented May 2, 2020 •

edited

Loading

shred86 commented May 8, 2020 •

edited

Loading

jgilliam17 commented Oct 20, 2020 •

edited

Loading