-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Fix freezes when previewing a large PDF file #22760
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
Changes from 7 commits
1996119
e0d3535
1257ab3
2c90716
8be7ec1
ae1ab8a
d8b5873
c95c775
7c190a5
d1939a9
b1fcbb8
fc2d10d
b35b11b
1cbbe74
a151a1c
717f296
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
import _ from 'underscore'; | ||
import React, {Component} from 'react'; | ||
import {View, Dimensions} from 'react-native'; | ||
import {View} from 'react-native'; | ||
import {Document, Page, pdfjs} from 'react-pdf/dist/esm/entry.webpack'; | ||
import pdfWorkerSource from 'pdfjs-dist/legacy/build/pdf.worker'; | ||
import {VariableSizeList as List} from 'react-window'; | ||
import FullScreenLoadingIndicator from '../FullscreenLoadingIndicator'; | ||
import styles from '../../styles/styles'; | ||
import variables from '../../styles/variables'; | ||
|
@@ -14,12 +15,20 @@ import withLocalize from '../withLocalize'; | |
import Text from '../Text'; | ||
import compose from '../../libs/compose'; | ||
|
||
/** | ||
* Each page has a default border. The app should take this size into account | ||
* when calculates the page width and height. | ||
*/ | ||
const PAGE_BORDER = 9; | ||
|
||
class PDFView extends Component { | ||
constructor(props) { | ||
super(props); | ||
this.state = { | ||
numPages: null, | ||
windowWidth: Dimensions.get('window').width, | ||
pageViewports: [], | ||
containerWidth: props.windowWidth, | ||
containerHeight: props.windowHeight, | ||
shouldRequestPassword: false, | ||
isPasswordInvalid: false, | ||
isKeyboardOpen: false, | ||
|
@@ -28,6 +37,9 @@ class PDFView extends Component { | |
this.initiatePasswordChallenge = this.initiatePasswordChallenge.bind(this); | ||
this.attemptPDFLoad = this.attemptPDFLoad.bind(this); | ||
this.toggleKeyboardOnSmallScreens = this.toggleKeyboardOnSmallScreens.bind(this); | ||
this.calculatePageHeight = this.calculatePageHeight.bind(this); | ||
this.calculatePageWidth = this.calculatePageWidth.bind(this); | ||
this.renderPage = this.renderPage.bind(this); | ||
|
||
const workerBlob = new Blob([pdfWorkerSource], {type: 'text/javascript'}); | ||
pdfjs.GlobalWorkerOptions.workerSrc = URL.createObjectURL(workerBlob); | ||
|
@@ -45,21 +57,66 @@ class PDFView extends Component { | |
} | ||
|
||
/** | ||
* Upon successful document load, set the number of pages on PDF, | ||
* Upon successful document load, combine an array of page viewports, | ||
* set the number of pages on PDF, | ||
* hide/reset PDF password form, and notify parent component that | ||
* user input is no longer required. | ||
* | ||
* @param {*} {numPages} No of pages in the rendered PDF | ||
* @param {*} pdf The PDF file instance | ||
* @memberof PDFView | ||
*/ | ||
onDocumentLoadSuccess({numPages}) { | ||
this.setState({ | ||
numPages, | ||
shouldRequestPassword: false, | ||
isPasswordInvalid: false, | ||
onDocumentLoadSuccess(pdf) { | ||
const numPages = pdf.numPages; | ||
|
||
Promise.all( | ||
_.times(numPages, (index) => { | ||
const pageNumber = index + 1; | ||
|
||
return pdf.getPage(pageNumber).then((page) => page.getViewport({scale: 1})); | ||
}), | ||
).then((pageViewports) => { | ||
this.setState({ | ||
pageViewports, | ||
numPages, | ||
shouldRequestPassword: false, | ||
isPasswordInvalid: false, | ||
}); | ||
}); | ||
} | ||
|
||
/** | ||
* Calculates a proper page height. | ||
* It is based on a ratio between the specific page viewport width and provided page width. | ||
* Also, the app should take into account the page borders. | ||
* @param {*} pageIndex | ||
* @returns {Number} | ||
*/ | ||
calculatePageHeight(pageIndex) { | ||
if (this.state.pageViewports.length === 0) { | ||
throw new Error('calculatePageHeight() called too early'); | ||
rezkiy37 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
const pageViewport = this.state.pageViewports[pageIndex]; | ||
const pageWidth = this.calculatePageWidth(); | ||
const scale = pageWidth / pageViewport.width; | ||
const actualHeight = pageViewport.height * scale + PAGE_BORDER * 2; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like we're using the virtual list concept here. Just out of curiosity, what's stopping us from using this solution on native? If I'm not wrong, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIK:
The libraries web and native are using are completely different.
FlatList can only be used on web combined with react-native-web. As react-pdf is a pure web library (not depending on react-native-web), no need to integrate with FlatList. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In short, |
||
|
||
return actualHeight; | ||
} | ||
|
||
/** | ||
* Calculates a proper page width. | ||
* It depends on a screen size. Also, the app should take into account the page borders. | ||
* @returns {Number} | ||
*/ | ||
calculatePageWidth() { | ||
const pdfContainerWidth = this.state.containerWidth; | ||
const pageWidthOnLargeScreen = pdfContainerWidth <= variables.pdfPageMaxWidth ? pdfContainerWidth : variables.pdfPageMaxWidth; | ||
rezkiy37 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const pageWidth = this.props.isSmallScreenWidth ? this.state.containerWidth : pageWidthOnLargeScreen; | ||
|
||
return pageWidth + PAGE_BORDER * 2; | ||
} | ||
|
||
/** | ||
* Initiate password challenge process. The react-pdf/Document | ||
* component calls this handler to indicate that a PDF requires a | ||
|
@@ -105,10 +162,32 @@ class PDFView extends Component { | |
this.props.onToggleKeyboard(isKeyboardOpen); | ||
} | ||
|
||
/** | ||
* Renders a specific page based on its index. | ||
* It includes a wrapper to apply virtualized styles. | ||
* @param {Number} index | ||
* @param {Object} style | ||
* @returns {JSX.Element} | ||
*/ | ||
renderPage({index, style}) { | ||
const pageWidth = this.calculatePageWidth(); | ||
|
||
return ( | ||
<View style={style}> | ||
<Page | ||
key={`page_${index}`} | ||
width={pageWidth} | ||
pageIndex={index} | ||
// This needs to be empty to avoid multiple loading texts which show per page and look ugly | ||
// See https://github.com/Expensify/App/issues/14358 for more details | ||
loading="" | ||
/> | ||
</View> | ||
); | ||
} | ||
|
||
render() { | ||
const pdfContainerWidth = this.state.windowWidth - 100; | ||
const pageWidthOnLargeScreen = pdfContainerWidth <= variables.pdfPageMaxWidth ? pdfContainerWidth : variables.pdfPageMaxWidth; | ||
const pageWidth = this.props.isSmallScreenWidth ? this.state.windowWidth : pageWidthOnLargeScreen; | ||
const pageWidth = this.calculatePageWidth(); | ||
rezkiy37 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const outerContainerStyle = [styles.w100, styles.h100, styles.justifyContentCenter, styles.alignItemsCenter]; | ||
|
||
// If we're requesting a password then we need to hide - but still render - | ||
|
@@ -122,7 +201,11 @@ class PDFView extends Component { | |
<View | ||
focusable | ||
style={pdfContainerStyle} | ||
onLayout={(event) => this.setState({windowWidth: event.nativeEvent.layout.width})} | ||
onLayout={({ | ||
nativeEvent: { | ||
layout: {width, height}, | ||
}, | ||
}) => this.setState({containerWidth: width, containerHeight: height})} | ||
> | ||
<Document | ||
error={<Text style={[styles.textLabel, styles.textLarge]}>{this.props.translate('attachmentView.failedToLoadPDF')}</Text>} | ||
|
@@ -136,16 +219,18 @@ class PDFView extends Component { | |
onLoadSuccess={this.onDocumentLoadSuccess} | ||
onPassword={this.initiatePasswordChallenge} | ||
> | ||
{_.map(_.range(this.state.numPages), (v, index) => ( | ||
<Page | ||
{this.state.pageViewports.length > 0 && ( | ||
<List | ||
style={styles.PDFViewList} | ||
width={pageWidth} | ||
key={`page_${index + 1}`} | ||
pageNumber={index + 1} | ||
// This needs to be empty to avoid multiple loading texts which show per page and look ugly | ||
// See https://github.com/Expensify/App/issues/14358 for more details | ||
loading="" | ||
/> | ||
))} | ||
height={this.state.containerHeight} | ||
estimatedItemSize={this.calculatePageHeight(0)} | ||
itemCount={this.state.numPages} | ||
itemSize={this.calculatePageHeight} | ||
> | ||
{this.renderPage} | ||
</List> | ||
)} | ||
</Document> | ||
</View> | ||
{this.state.shouldRequestPassword && ( | ||
|
Uh oh!
There was an error while loading. Please reload this page.