From 19961196237cdf79e36c13372971350fcfaac496 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 21 Jun 2023 13:16:20 +0200 Subject: [PATCH 1/9] Add react-window --- package-lock.json | 43 ++++++++++++++++++++++++++++++++++++------- package.json | 1 + 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0e83091a5474..3e8b85927585 100644 --- a/package-lock.json +++ b/package-lock.json @@ -96,6 +96,7 @@ "react-pdf": "5.7.2", "react-plaid-link": "3.3.2", "react-web-config": "^1.0.0", + "react-window": "^1.8.9", "save": "^2.4.0", "semver": "^7.3.8", "shim-keyboard-event-key": "^1.0.3", @@ -31801,6 +31802,11 @@ "node": ">= 4.0.0" } }, + "node_modules/memoize-one": { + "version": "5.2.1", + "resolved": "https://registry.npmjs.org/memoize-one/-/memoize-one-5.2.1.tgz", + "integrity": "sha512-zYiwtZUcYyXKo/np96AGZAckk+FWWsUdJ3cHGGmld7+AhvcWmQyGCYUh1hc4Q/pkOhb65dQR/pqCyK0cOaHz4Q==" + }, "node_modules/memoizerific": { "version": "1.11.3", "dev": true, @@ -36675,10 +36681,6 @@ "prop-types": "*" } }, - "node_modules/react-native/node_modules/memoize-one": { - "version": "5.2.1", - "license": "MIT" - }, "node_modules/react-native/node_modules/metro-runtime": { "version": "0.73.7", "resolved": "https://registry.npmjs.org/metro-runtime/-/metro-runtime-0.73.7.tgz", @@ -37290,6 +37292,22 @@ "react-dom": ">=16.2.0" } }, + "node_modules/react-window": { + "version": "1.8.9", + "resolved": "https://registry.npmjs.org/react-window/-/react-window-1.8.9.tgz", + "integrity": "sha512-+Eqx/fj1Aa5WnhRfj9dJg4VYATGwIUP2ItwItiJ6zboKWA6EX3lYDAXfGF2hyNqplEprhbtjbipiADEcwQ823Q==", + "dependencies": { + "@babel/runtime": "^7.0.0", + "memoize-one": ">=3.1.1 <6" + }, + "engines": { + "node": ">8.0.0" + }, + "peerDependencies": { + "react": "^15.0.0 || ^16.0.0 || ^17.0.0 || ^18.0.0", + "react-dom": "^15.0.0 || ^16.0.0 || ^17.0.0 || ^18.0.0" + } + }, "node_modules/read-config-file": { "version": "6.3.2", "resolved": "https://registry.npmjs.org/read-config-file/-/read-config-file-6.3.2.tgz", @@ -64389,6 +64407,11 @@ "fs-monkey": "^1.0.3" } }, + "memoize-one": { + "version": "5.2.1", + "resolved": "https://registry.npmjs.org/memoize-one/-/memoize-one-5.2.1.tgz", + "integrity": "sha512-zYiwtZUcYyXKo/np96AGZAckk+FWWsUdJ3cHGGmld7+AhvcWmQyGCYUh1hc4Q/pkOhb65dQR/pqCyK0cOaHz4Q==" + }, "memoizerific": { "version": "1.11.3", "dev": true, @@ -67331,9 +67354,6 @@ "prop-types": "*" } }, - "memoize-one": { - "version": "5.2.1" - }, "metro-runtime": { "version": "0.73.7", "resolved": "https://registry.npmjs.org/metro-runtime/-/metro-runtime-0.73.7.tgz", @@ -68098,6 +68118,15 @@ "integrity": "sha512-8E/Eb/7ksKwn5QdLn67tOR7+TdP9BZdu6E5/DSt20v8yfW/s0VGBigE6VA7R4278mBuBUowovAB3DkCfVmSPvA==", "requires": {} }, + "react-window": { + "version": "1.8.9", + "resolved": "https://registry.npmjs.org/react-window/-/react-window-1.8.9.tgz", + "integrity": "sha512-+Eqx/fj1Aa5WnhRfj9dJg4VYATGwIUP2ItwItiJ6zboKWA6EX3lYDAXfGF2hyNqplEprhbtjbipiADEcwQ823Q==", + "requires": { + "@babel/runtime": "^7.0.0", + "memoize-one": ">=3.1.1 <6" + } + }, "read-config-file": { "version": "6.3.2", "resolved": "https://registry.npmjs.org/read-config-file/-/read-config-file-6.3.2.tgz", diff --git a/package.json b/package.json index 0dbe21194334..650aefbb49cf 100644 --- a/package.json +++ b/package.json @@ -132,6 +132,7 @@ "react-pdf": "5.7.2", "react-plaid-link": "3.3.2", "react-web-config": "^1.0.0", + "react-window": "^1.8.9", "save": "^2.4.0", "semver": "^7.3.8", "shim-keyboard-event-key": "^1.0.3", From e0d35358805b6b2c6809da6f1447f0883f3edff3 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 21 Jun 2023 13:16:46 +0200 Subject: [PATCH 2/9] Fix previewing of large pdf files --- src/components/PDFView/index.js | 91 ++++++++++++++++++++++++++------- 1 file changed, 72 insertions(+), 19 deletions(-) diff --git a/src/components/PDFView/index.js b/src/components/PDFView/index.js index 85821b8d8ca3..9d37c26db199 100644 --- a/src/components/PDFView/index.js +++ b/src/components/PDFView/index.js @@ -3,6 +3,7 @@ import React, {Component} from 'react'; import {View, Dimensions} 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'; @@ -19,6 +20,7 @@ class PDFView extends Component { super(props); this.state = { numPages: null, + pageViewports: [], windowWidth: Dimensions.get('window').width, shouldRequestPassword: false, isPasswordInvalid: false, @@ -28,6 +30,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 +50,53 @@ 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, + }); }); } + calculatePageHeight(pageIndex) { + if (this.state.pageViewports.length === 0) { + throw new Error('calculatePageHeight() called too early'); + } + + const pageViewport = this.state.pageViewports[pageIndex]; + const scale = this.calculatePageWidth() / pageViewport.width; + const actualHeight = pageViewport.height * scale; + + return actualHeight; + } + + calculatePageWidth() { + const pdfContainerWidth = this.state.windowWidth - 100; + const pageWidthOnLargeScreen = pdfContainerWidth <= variables.pdfPageMaxWidth ? pdfContainerWidth : variables.pdfPageMaxWidth; + const pageWidth = this.props.isSmallScreenWidth ? this.state.windowWidth : pageWidthOnLargeScreen; + + return pageWidth; + } + /** * Initiate password challenge process. The react-pdf/Document * component calls this handler to indicate that a PDF requires a @@ -105,10 +142,25 @@ class PDFView extends Component { this.props.onToggleKeyboard(isKeyboardOpen); } + renderPage({index, style}) { + const pageWidth = this.calculatePageWidth(); + + return ( + + + + ); + } + 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(); const outerContainerStyle = [styles.w100, styles.h100, styles.justifyContentCenter, styles.alignItemsCenter]; // If we're requesting a password then we need to hide - but still render - @@ -136,16 +188,17 @@ class PDFView extends Component { onLoadSuccess={this.onDocumentLoadSuccess} onPassword={this.initiatePasswordChallenge} > - {_.map(_.range(this.state.numPages), (v, index) => ( - 0 && ( + - ))} + height={this.props.windowHeight} + estimatedItemSize={this.calculatePageHeight(0)} + itemCount={this.state.numPages} + itemSize={this.calculatePageHeight} + > + {this.renderPage} + + )} {this.state.shouldRequestPassword && ( From ae1ab8a3e25359a269582edcc6e7511e74d0a8d9 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Wed, 12 Jul 2023 17:03:45 +0200 Subject: [PATCH 3/9] fix dimensions --- src/components/PDFView/index.js | 26 ++++++++++++++++++-------- src/styles/styles.js | 5 ++++- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/components/PDFView/index.js b/src/components/PDFView/index.js index 357be7f8f40b..7d9da52a06b3 100644 --- a/src/components/PDFView/index.js +++ b/src/components/PDFView/index.js @@ -1,6 +1,6 @@ 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'; @@ -15,13 +15,16 @@ import withLocalize from '../withLocalize'; import Text from '../Text'; import compose from '../../libs/compose'; +const PAGE_BORDER = 9; + class PDFView extends Component { constructor(props) { super(props); this.state = { numPages: null, pageViewports: [], - windowWidth: Dimensions.get('window').width, + containerWidth: props.windowWidth, + containerHeight: props.windowHeight, shouldRequestPassword: false, isPasswordInvalid: false, isKeyboardOpen: false, @@ -77,6 +80,7 @@ class PDFView extends Component { }); } + // TODO: Add a comment calculatePageHeight(pageIndex) { if (this.state.pageViewports.length === 0) { throw new Error('calculatePageHeight() called too early'); @@ -84,17 +88,18 @@ class PDFView extends Component { const pageViewport = this.state.pageViewports[pageIndex]; const scale = this.calculatePageWidth() / pageViewport.width; - const actualHeight = pageViewport.height * scale; + const actualHeight = pageViewport.height * scale + PAGE_BORDER * 2; return actualHeight; } + // TODO: Add a comment calculatePageWidth() { - const pdfContainerWidth = this.state.windowWidth - 100; + const pdfContainerWidth = this.state.containerWidth; const pageWidthOnLargeScreen = pdfContainerWidth <= variables.pdfPageMaxWidth ? pdfContainerWidth : variables.pdfPageMaxWidth; - const pageWidth = this.props.isSmallScreenWidth ? this.state.windowWidth : pageWidthOnLargeScreen; + const pageWidth = this.props.isSmallScreenWidth ? this.state.containerWidth : pageWidthOnLargeScreen; - return pageWidth; + return pageWidth + PAGE_BORDER * 2; } /** @@ -174,7 +179,11 @@ class PDFView extends Component { this.setState({windowWidth: event.nativeEvent.layout.width})} + onLayout={({ + nativeEvent: { + layout: {width, height}, + }, + }) => this.setState({containerWidth: width, containerHeight: height})} > {this.props.translate('attachmentView.failedToLoadPDF')}} @@ -190,8 +199,9 @@ class PDFView extends Component { > {this.state.pageViewports.length > 0 && ( Date: Wed, 12 Jul 2023 19:10:51 +0200 Subject: [PATCH 4/9] add JSDoc --- src/components/PDFView/index.js | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/components/PDFView/index.js b/src/components/PDFView/index.js index 7d9da52a06b3..b51796e276fc 100644 --- a/src/components/PDFView/index.js +++ b/src/components/PDFView/index.js @@ -15,6 +15,10 @@ 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 { @@ -80,20 +84,31 @@ class PDFView extends Component { }); } - // TODO: Add a comment + /** + * 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'); } const pageViewport = this.state.pageViewports[pageIndex]; - const scale = this.calculatePageWidth() / pageViewport.width; + const pageWidth = this.calculatePageWidth(); + const scale = pageWidth / pageViewport.width; const actualHeight = pageViewport.height * scale + PAGE_BORDER * 2; return actualHeight; } - // TODO: Add a comment + /** + * 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; @@ -147,6 +162,13 @@ 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(); From c95c7754f11fafd9ff745a339c99fd2510e681a2 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Thu, 13 Jul 2023 11:20:26 +0200 Subject: [PATCH 5/9] improve PDFView methods --- src/components/PDFView/index.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/components/PDFView/index.js b/src/components/PDFView/index.js index b51796e276fc..49130013d88a 100644 --- a/src/components/PDFView/index.js +++ b/src/components/PDFView/index.js @@ -14,6 +14,7 @@ import withWindowDimensions from '../withWindowDimensions'; import withLocalize from '../withLocalize'; import Text from '../Text'; import compose from '../../libs/compose'; +import Log from '../../libs/Log'; /** * Each page has a default border. The app should take this size into account @@ -85,7 +86,7 @@ class PDFView extends Component { } /** - * Calculates a proper page height. + * Calculates a proper page height. The method should be called only when there are page viewports. * 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 @@ -93,7 +94,9 @@ class PDFView extends Component { */ calculatePageHeight(pageIndex) { if (this.state.pageViewports.length === 0) { - throw new Error('calculatePageHeight() called too early'); + Log.warn('Dev error: calculatePageHeight() in PDFView called too early'); + + return 0; } const pageViewport = this.state.pageViewports[pageIndex]; @@ -111,7 +114,7 @@ class PDFView extends Component { */ calculatePageWidth() { const pdfContainerWidth = this.state.containerWidth; - const pageWidthOnLargeScreen = pdfContainerWidth <= variables.pdfPageMaxWidth ? pdfContainerWidth : variables.pdfPageMaxWidth; + const pageWidthOnLargeScreen = Math.min(pdfContainerWidth, variables.pdfPageMaxWidth); const pageWidth = this.props.isSmallScreenWidth ? this.state.containerWidth : pageWidthOnLargeScreen; return pageWidth + PAGE_BORDER * 2; From 7c190a51fbf2fbbed489c1f1fa94813428f0c27f Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Thu, 13 Jul 2023 11:51:05 +0200 Subject: [PATCH 6/9] use currying --- src/components/PDFView/index.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/components/PDFView/index.js b/src/components/PDFView/index.js index 49130013d88a..2949ae4a51ee 100644 --- a/src/components/PDFView/index.js +++ b/src/components/PDFView/index.js @@ -166,16 +166,13 @@ class PDFView extends Component { } /** - * Renders a specific page based on its index. - * It includes a wrapper to apply virtualized styles. - * @param {Number} index - * @param {Object} style + * It is a currying method that returns a function that renders a specific page based on its index. + * The function includes a wrapper to apply virtualized styles. + * @param {Number} pageWidth * @returns {JSX.Element} */ - renderPage({index, style}) { - const pageWidth = this.calculatePageWidth(); - - return ( + renderPage(pageWidth) { + return ({index, style}) => ( - {this.renderPage} + {this.renderPage(pageWidth)} )} From fc2d10df322669d04dae9be23b362f280e00a718 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Tue, 25 Jul 2023 15:04:37 +0200 Subject: [PATCH 7/9] fix width of list and pages --- src/components/PDFView/index.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/components/PDFView/index.js b/src/components/PDFView/index.js index 6181116391f3..be7075610d14 100644 --- a/src/components/PDFView/index.js +++ b/src/components/PDFView/index.js @@ -22,6 +22,11 @@ import Log from '../../libs/Log'; * when calculates the page width and height. */ const PAGE_BORDER = 9; +/** + * Pages should be more narrow than the container on large screens. The app should take this size into account + * when calculates the page width. + */ +const LARGE_SCREEN_SIDE_SPACING = 40; class PDFView extends Component { constructor(props) { @@ -118,7 +123,7 @@ class PDFView extends Component { */ calculatePageWidth() { const pdfContainerWidth = this.state.containerWidth; - const pageWidthOnLargeScreen = Math.min(pdfContainerWidth, variables.pdfPageMaxWidth); + const pageWidthOnLargeScreen = Math.min(pdfContainerWidth - LARGE_SCREEN_SIDE_SPACING * 2, variables.pdfPageMaxWidth); const pageWidth = this.props.isSmallScreenWidth ? this.state.containerWidth : pageWidthOnLargeScreen; return pageWidth + PAGE_BORDER * 2; @@ -226,7 +231,7 @@ class PDFView extends Component { {this.state.pageViewports.length > 0 && ( Date: Mon, 7 Aug 2023 12:57:52 +0200 Subject: [PATCH 8/9] simplify onDocumentLoadSuccess --- src/components/PDFView/index.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/PDFView/index.js b/src/components/PDFView/index.js index b4bdd6a0bfd6..e7110e01611b 100644 --- a/src/components/PDFView/index.js +++ b/src/components/PDFView/index.js @@ -73,17 +73,17 @@ class PDFView extends Component { * hide/reset PDF password form, and notify parent component that * user input is no longer required. * - * @param {*} pdf The PDF file instance + * @param {Object} pdf - The PDF file instance + * @param {Number} pdf.numPages - Number of pages of the PDF file + * @param {Function} pdf.getPage - A method to get page by its number * @memberof PDFView */ - onDocumentLoadSuccess(pdf) { - const numPages = pdf.numPages; - + onDocumentLoadSuccess({numPages, getPage}) { Promise.all( _.times(numPages, (index) => { const pageNumber = index + 1; - return pdf.getPage(pageNumber).then((page) => page.getViewport({scale: 1})); + return getPage(pageNumber).then((page) => page.getViewport({scale: 1})); }), ).then((pageViewports) => { this.setState({ From 717f2961b53d9d138db07c204c377b65777565c7 Mon Sep 17 00:00:00 2001 From: Mykhailo Kravchenko Date: Mon, 7 Aug 2023 13:20:09 +0200 Subject: [PATCH 9/9] fix --- src/components/PDFView/index.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/PDFView/index.js b/src/components/PDFView/index.js index e7110e01611b..75c99757e7e0 100644 --- a/src/components/PDFView/index.js +++ b/src/components/PDFView/index.js @@ -75,15 +75,17 @@ class PDFView extends Component { * * @param {Object} pdf - The PDF file instance * @param {Number} pdf.numPages - Number of pages of the PDF file - * @param {Function} pdf.getPage - A method to get page by its number + * @param {Function} pdf.getPage - A method to get page by its number. It requires to have the context. It should be the pdf itself. * @memberof PDFView */ - onDocumentLoadSuccess({numPages, getPage}) { + onDocumentLoadSuccess(pdf) { + const {numPages} = pdf; + Promise.all( _.times(numPages, (index) => { const pageNumber = index + 1; - return getPage(pageNumber).then((page) => page.getViewport({scale: 1})); + return pdf.getPage(pageNumber).then((page) => page.getViewport({scale: 1})); }), ).then((pageViewports) => { this.setState({