From d764d50ba913465f940e2a2613c8c79e611b9761 Mon Sep 17 00:00:00 2001 From: hublot Date: Sat, 13 May 2023 00:39:22 +0800 Subject: [PATCH 1/3] Remove borderWidth of fullScreen smallScreen modal and modify PDF width --- src/components/AttachmentCarousel/index.js | 6 +++--- src/components/PDFView/index.js | 5 +++-- src/styles/getModalStyles/getBaseModalStyles.js | 8 ++++---- src/styles/styles.js | 4 ++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/components/AttachmentCarousel/index.js b/src/components/AttachmentCarousel/index.js index b9315894a4b4..1ef7a6fe906e 100644 --- a/src/components/AttachmentCarousel/index.js +++ b/src/components/AttachmentCarousel/index.js @@ -244,8 +244,8 @@ class AttachmentCarousel extends React.Component { renderCell(props) { // Use window width instead of layout width to address the issue in https://github.com/Expensify/App/issues/17760 // considering horizontal margin and border width in centered modal - const modalStyles = styles.centeredModalStyles(this.props.isSmallScreenWidth); - const style = [props.style, styles.h100, {width: this.props.windowWidth - (modalStyles.marginHorizontal + modalStyles.borderWidth) * 2 + 1}]; + const modalStyles = styles.centeredModalStyles(this.props.isSmallScreenWidth, true); + const style = [props.style, styles.h100, {width: this.props.windowWidth - (modalStyles.marginHorizontal + modalStyles.borderWidth) * 2}]; return ( this.setState({containerWidth: nativeEvent.layout.width + 1})} + onLayout={({nativeEvent}) => this.setState({containerWidth: nativeEvent.layout.width})} onMouseEnter={() => !this.canUseTouchScreen && this.toggleArrowsVisibility(true)} onMouseLeave={() => !this.canUseTouchScreen && this.toggleArrowsVisibility(false)} > diff --git a/src/components/PDFView/index.js b/src/components/PDFView/index.js index 77886ff512f2..2027ce501af3 100644 --- a/src/components/PDFView/index.js +++ b/src/components/PDFView/index.js @@ -108,8 +108,9 @@ class PDFView extends Component { render() { const pdfContainerWidth = this.state.windowWidth - 100; const pageWidthOnLargeScreen = pdfContainerWidth <= variables.pdfPageMaxWidth ? pdfContainerWidth : variables.pdfPageMaxWidth; - const pageWidth = this.props.isSmallScreenWidth ? this.state.windowWidth - 30 : pageWidthOnLargeScreen; - + // On android_chrome, the actual display width of the pdf will be slightly larger than pageWidth, maybe the react-pdf canvas decimal calculation is incorrect, + // or maybe because of window.visualViewport.width - window.innerWidth = 0.090911865234375, so we reduce it by 1px + const pageWidth = this.props.isSmallScreenWidth ? this.state.windowWidth - 1 : pageWidthOnLargeScreen; const outerContainerStyle = [styles.w100, styles.h100, styles.justifyContentCenter, styles.alignItemsCenter]; // If we're requesting a password then we need to hide - but still render - diff --git a/src/styles/getModalStyles/getBaseModalStyles.js b/src/styles/getModalStyles/getBaseModalStyles.js index ddc926b3ce09..9f37642b9b25 100644 --- a/src/styles/getModalStyles/getBaseModalStyles.js +++ b/src/styles/getModalStyles/getBaseModalStyles.js @@ -3,8 +3,8 @@ import variables from '../variables'; import themeColors from '../themes/default'; import styles from '../styles'; -const getCenteredModalStyles = (windowWidth, isSmallScreenWidth) => ({ - borderWidth: styles.centeredModalStyles(isSmallScreenWidth).borderWidth, +const getCenteredModalStyles = (windowWidth, isSmallScreenWidth, isFullScreenWhenSmall) => ({ + borderWidth: styles.centeredModalStyles(isSmallScreenWidth, isFullScreenWhenSmall).borderWidth, width: isSmallScreenWidth ? '100%' : windowWidth - styles.centeredModalStyles(isSmallScreenWidth).marginHorizontal * 2, }); @@ -83,7 +83,7 @@ export default (type, windowDimensions, popoverAnchorPosition = {}, innerContain marginBottom: isSmallScreenWidth ? 0 : 20, borderRadius: isSmallScreenWidth ? 0 : 12, overflow: 'hidden', - ...getCenteredModalStyles(windowWidth, isSmallScreenWidth), + ...getCenteredModalStyles(windowWidth, isSmallScreenWidth, false), }; // Allow this modal to be dismissed with a swipe down or swipe right @@ -118,7 +118,7 @@ export default (type, windowDimensions, popoverAnchorPosition = {}, innerContain marginBottom: isSmallScreenWidth ? 0 : 20, borderRadius: isSmallScreenWidth ? 0 : 12, overflow: 'hidden', - ...getCenteredModalStyles(windowWidth, isSmallScreenWidth), + ...getCenteredModalStyles(windowWidth, isSmallScreenWidth, true), }; swipeDirection = undefined; animationIn = isSmallScreenWidth ? 'slideInRight' : 'fadeIn'; diff --git a/src/styles/styles.js b/src/styles/styles.js index 9749b0cbd324..c7228c2a2f69 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -1977,8 +1977,8 @@ const styles = { backgroundColor: themeColors.modalBackdrop, }, - centeredModalStyles: (isSmallScreenWidth) => ({ - borderWidth: isSmallScreenWidth ? 1 : 0, + centeredModalStyles: (isSmallScreenWidth, isFullScreenWhenSmall) => ({ + borderWidth: isSmallScreenWidth && !isFullScreenWhenSmall ? 1 : 0, marginHorizontal: isSmallScreenWidth ? 0 : 20, }), From 927f03bb928504f7e04d3bb9353c2e3dab808c55 Mon Sep 17 00:00:00 2001 From: hublot Date: Sun, 14 May 2023 00:08:15 +0800 Subject: [PATCH 2/3] Add roundToNearestPixel to solve android chrome contentOffset --- src/components/AttachmentCarousel/index.js | 6 +++--- src/components/PDFView/index.js | 4 +--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/components/AttachmentCarousel/index.js b/src/components/AttachmentCarousel/index.js index 1ef7a6fe906e..380f865394ce 100644 --- a/src/components/AttachmentCarousel/index.js +++ b/src/components/AttachmentCarousel/index.js @@ -1,5 +1,5 @@ import React from 'react'; -import {View, FlatList} from 'react-native'; +import {View, FlatList, PixelRatio} from 'react-native'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; @@ -245,7 +245,7 @@ class AttachmentCarousel extends React.Component { // Use window width instead of layout width to address the issue in https://github.com/Expensify/App/issues/17760 // considering horizontal margin and border width in centered modal const modalStyles = styles.centeredModalStyles(this.props.isSmallScreenWidth, true); - const style = [props.style, styles.h100, {width: this.props.windowWidth - (modalStyles.marginHorizontal + modalStyles.borderWidth) * 2}]; + const style = [props.style, styles.h100, {width: PixelRatio.roundToNearestPixel(this.props.windowWidth - (modalStyles.marginHorizontal + modalStyles.borderWidth) * 2)}]; return ( this.setState({containerWidth: nativeEvent.layout.width})} + onLayout={({nativeEvent}) => this.setState({containerWidth: PixelRatio.roundToNearestPixel(nativeEvent.layout.width)})} onMouseEnter={() => !this.canUseTouchScreen && this.toggleArrowsVisibility(true)} onMouseLeave={() => !this.canUseTouchScreen && this.toggleArrowsVisibility(false)} > diff --git a/src/components/PDFView/index.js b/src/components/PDFView/index.js index 2027ce501af3..85821b8d8ca3 100644 --- a/src/components/PDFView/index.js +++ b/src/components/PDFView/index.js @@ -108,9 +108,7 @@ class PDFView extends Component { render() { const pdfContainerWidth = this.state.windowWidth - 100; const pageWidthOnLargeScreen = pdfContainerWidth <= variables.pdfPageMaxWidth ? pdfContainerWidth : variables.pdfPageMaxWidth; - // On android_chrome, the actual display width of the pdf will be slightly larger than pageWidth, maybe the react-pdf canvas decimal calculation is incorrect, - // or maybe because of window.visualViewport.width - window.innerWidth = 0.090911865234375, so we reduce it by 1px - const pageWidth = this.props.isSmallScreenWidth ? this.state.windowWidth - 1 : pageWidthOnLargeScreen; + const pageWidth = this.props.isSmallScreenWidth ? this.state.windowWidth : pageWidthOnLargeScreen; const outerContainerStyle = [styles.w100, styles.h100, styles.justifyContentCenter, styles.alignItemsCenter]; // If we're requesting a password then we need to hide - but still render - From c32e848b3f08c3646e8b33740a0716a0c878f63a Mon Sep 17 00:00:00 2001 From: hublot Date: Mon, 15 May 2023 07:14:02 +0800 Subject: [PATCH 3/3] Add isFullScreenWhenSmall parameter default value --- src/styles/getModalStyles/getBaseModalStyles.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/styles/getModalStyles/getBaseModalStyles.js b/src/styles/getModalStyles/getBaseModalStyles.js index 9f37642b9b25..b7a3317963ca 100644 --- a/src/styles/getModalStyles/getBaseModalStyles.js +++ b/src/styles/getModalStyles/getBaseModalStyles.js @@ -3,9 +3,9 @@ import variables from '../variables'; import themeColors from '../themes/default'; import styles from '../styles'; -const getCenteredModalStyles = (windowWidth, isSmallScreenWidth, isFullScreenWhenSmall) => ({ +const getCenteredModalStyles = (windowWidth, isSmallScreenWidth, isFullScreenWhenSmall = false) => ({ borderWidth: styles.centeredModalStyles(isSmallScreenWidth, isFullScreenWhenSmall).borderWidth, - width: isSmallScreenWidth ? '100%' : windowWidth - styles.centeredModalStyles(isSmallScreenWidth).marginHorizontal * 2, + width: isSmallScreenWidth ? '100%' : windowWidth - styles.centeredModalStyles(isSmallScreenWidth, isFullScreenWhenSmall).marginHorizontal * 2, }); export default (type, windowDimensions, popoverAnchorPosition = {}, innerContainerStyle = {}, outerStyle = {}) => { @@ -83,7 +83,7 @@ export default (type, windowDimensions, popoverAnchorPosition = {}, innerContain marginBottom: isSmallScreenWidth ? 0 : 20, borderRadius: isSmallScreenWidth ? 0 : 12, overflow: 'hidden', - ...getCenteredModalStyles(windowWidth, isSmallScreenWidth, false), + ...getCenteredModalStyles(windowWidth, isSmallScreenWidth), }; // Allow this modal to be dismissed with a swipe down or swipe right