Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Lazy loading pdf.js library #9371

Merged
merged 3 commits into from
Nov 29, 2021
Merged

Lazy loading pdf.js library #9371

merged 3 commits into from
Nov 29, 2021

Conversation

jespino
Copy link
Member

@jespino jespino commented Nov 12, 2021 •
edited
Loading

Summary

This adds lazy loading to the pdf.js library, that is used only for previewing pdf but it is a heavy library. Also it changes how is exposed to the plugins to make it lazy.

Ticket Link

MM-40323

Release Note

NONE

@jespino jespino added the 2: Dev Review Requires review by a core commiter label Nov 12, 2021
hmhealey
hmhealey reviewed Nov 15, 2021
View reviewed changes
@jespino jespino requested a review from hmhealey November 17, 2021 11:33
hmhealey
hmhealey approved these changes Nov 23, 2021
View reviewed changes
Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

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

Cool. I'm all for exporting less stuff to plugins since it's less stuff for us to have to support while we get a better system figured out for it 😀

@hmhealey hmhealey added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Nov 23, 2021
@hmhealey
Copy link
Member

Is this something we'd want QA to test? We could defer QA testing for it, but I think that requires a Jira ticket so that QA know they have something to test

@jespino
Copy link
Member Author

jespino commented Nov 24, 2021

@hmhealey yes, I think at least we should test the case where the user open a PDF file preview.

@jespino jespino added 3: QA Review Requires review by a QA tester and removed 4: Reviews Complete All reviewers have approved the pull request labels Nov 24, 2021
@jespino jespino requested review from furqanmlk and jgilliam17 and removed request for furqanmlk November 24, 2021 10:36
@jgilliam17 jgilliam17 added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Nov 29, 2021
@jgilliam17
Copy link
Contributor

Thanks @jespino
Here's the E2E report -looks good, no related failures.
Manually tested PDF file preview, works as before - looks good to merge.

@jgilliam17
Copy link
Contributor

/update-branch

@jgilliam17 jgilliam17 merged commit 887a77b into mattermost:master Nov 29, 2021
@mattermod mattermod removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Nov 29, 2021
@mm-cloud-bot
Copy link

Test server destroyed

1 similar comment
@mm-cloud-bot
Copy link

Test server destroyed

@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Dec 1, 2021
cleferman pushed a commit to cleferman/mattermost-webapp that referenced this pull request Dec 21, 2021
@jespino @mattermod
Lazy loading pdf.js library (mattermost#9371)
e32c016 
* Lazy loading pdf.js library

* Removing pdfjs lib from plugins/export

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@M-ZubairAhmed M-ZubairAhmed M-ZubairAhmed approved these changes

@hmhealey hmhealey hmhealey approved these changes

@jgilliam17 jgilliam17 jgilliam17 approved these changes

Assignees

@jespino jespino

Labels
3: QA Review Requires review by a QA tester Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation release-note-none
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jespino @hmhealey @jgilliam17 @mm-cloud-bot @M-ZubairAhmed @mattermod @amyblais