-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix unable to scroll PDF files in preview #21714
Changes from all commits
d47bb44
974747f
23152ad
fac115a
01d73e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,11 +7,13 @@ import * as StyleUtils from '../../styles/StyleUtils'; | |
import FullScreenLoadingIndicator from '../FullscreenLoadingIndicator'; | ||
import Text from '../Text'; | ||
import PDFPasswordForm from './PDFPasswordForm'; | ||
import PressableWithoutFeedback from '../Pressable/PressableWithoutFeedback'; | ||
import {propTypes as pdfViewPropTypes, defaultProps} from './pdfViewPropTypes'; | ||
import compose from '../../libs/compose'; | ||
import withWindowDimensions from '../withWindowDimensions'; | ||
import withKeyboardState, {keyboardStatePropTypes} from '../withKeyboardState'; | ||
import withLocalize from '../withLocalize'; | ||
import CONST from '../../CONST'; | ||
|
||
const propTypes = { | ||
...pdfViewPropTypes, | ||
|
@@ -41,6 +43,7 @@ class PDFView extends Component { | |
shouldShowLoadingIndicator: true, | ||
isPasswordInvalid: false, | ||
failedToLoadPDF: false, | ||
successToLoadPDF: false, | ||
password: '', | ||
}; | ||
this.initiatePasswordChallenge = this.initiatePasswordChallenge.bind(this); | ||
|
@@ -118,11 +121,12 @@ class PDFView extends Component { | |
this.setState({ | ||
shouldRequestPassword: false, | ||
shouldShowLoadingIndicator: false, | ||
successToLoadPDF: true, | ||
}); | ||
this.props.onLoadComplete(); | ||
} | ||
|
||
render() { | ||
renderPDFView() { | ||
const pdfStyles = [styles.imageModalPDF, StyleUtils.getWidthAndHeightStyle(this.props.windowWidth, this.props.windowHeight)]; | ||
|
||
// If we haven't yet successfully validated the password and loaded the PDF, | ||
|
@@ -169,6 +173,21 @@ class PDFView extends Component { | |
</View> | ||
); | ||
} | ||
|
||
render() { | ||
return this.props.onPress && !this.state.successToLoadPDF ? ( | ||
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. Adding this condition introduced #23475 bug. 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. This condition caused the #42103 (comment) bug. We should've disabled the component instead of conditional rendering. |
||
<PressableWithoutFeedback | ||
onPress={this.props.onPress} | ||
style={[styles.flex1, styles.flexRow, styles.alignSelfStretch]} | ||
accessibilityRole={CONST.ACCESSIBILITY_ROLE.IMAGEBUTTON} | ||
accessibilityLabel={this.props.fileName || this.props.translate('attachmentView.unknownFilename')} | ||
> | ||
{this.renderPDFView()} | ||
</PressableWithoutFeedback> | ||
) : ( | ||
this.renderPDFView() | ||
); | ||
} | ||
} | ||
|
||
PDFView.propTypes = propTypes; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding
styles.flexRow
caused a regression where failed to load message is not properly visible on native #21714