From b692714b2fb99db63177e23ffd76c567f0e5d3ab Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Tue, 27 Feb 2024 01:31:49 +0700 Subject: [PATCH 1/9] fix virtual viewport scroll on report screen --- src/components/EmojiPicker/EmojiPicker.js | 2 +- .../EmojiPickerMenu/useEmojiPickerMenu.js | 4 +- .../useViewportOffsetTop/index.native.ts | 7 + src/hooks/useViewportOffsetTop/index.ts | 51 ++++++ src/libs/actions/Modal.ts | 1 + src/pages/home/ReportScreen.tsx | 160 +++++++++--------- .../ReportActionCompose.tsx | 20 ++- src/pages/home/report/ReportFooter.tsx | 9 + 8 files changed, 172 insertions(+), 82 deletions(-) create mode 100644 src/hooks/useViewportOffsetTop/index.native.ts create mode 100644 src/hooks/useViewportOffsetTop/index.ts diff --git a/src/components/EmojiPicker/EmojiPicker.js b/src/components/EmojiPicker/EmojiPicker.js index 6bceaf570ccc..fd0fe786b632 100644 --- a/src/components/EmojiPicker/EmojiPicker.js +++ b/src/components/EmojiPicker/EmojiPicker.js @@ -176,7 +176,7 @@ const EmojiPicker = forwardRef((props, ref) => { onModalShow={focusEmojiSearchInput} onModalHide={onModalHide.current} hideModalContentWhileAnimating - shouldSetModalVisibility={false} + shouldSetModalVisibility={Browser.isMobile()} animationInTiming={1} animationOutTiming={1} anchorPosition={{ diff --git a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js index 7caab5e6fb55..80521e04ddd9 100644 --- a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js +++ b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js @@ -22,9 +22,9 @@ const useEmojiPickerMenu = () => { const isListFiltered = allEmojis.length !== filteredEmojis.length; const {preferredLocale} = useLocalize(); const [preferredSkinTone] = usePreferredEmojiSkinTone(); - const {windowHeight} = useWindowDimensions(); + const {windowHeight} = useWindowDimensions(true); const StyleUtils = useStyleUtils(); - const listStyle = StyleUtils.getEmojiPickerListHeight(isListFiltered, windowHeight); + const listStyle = StyleUtils.getEmojiPickerListHeight(isListFiltered, windowHeight * 0.95); useEffect(() => { setFilteredEmojis(allEmojis); diff --git a/src/hooks/useViewportOffsetTop/index.native.ts b/src/hooks/useViewportOffsetTop/index.native.ts new file mode 100644 index 000000000000..b166c360dacc --- /dev/null +++ b/src/hooks/useViewportOffsetTop/index.native.ts @@ -0,0 +1,7 @@ +/** + * Native doesn't support DOM so default value is 0 + */ + +export default function useViewportOffsetTop(): number { + return 0; +} diff --git a/src/hooks/useViewportOffsetTop/index.ts b/src/hooks/useViewportOffsetTop/index.ts new file mode 100644 index 000000000000..7bb13f55c5ee --- /dev/null +++ b/src/hooks/useViewportOffsetTop/index.ts @@ -0,0 +1,51 @@ +import {useEffect, useRef, useState} from 'react'; +import addViewportResizeListener from '@libs/VisualViewport'; + +/** + * A hook that returns the offset of the top edge of the visual viewport + */ +export default function useViewportOffsetTop(shouldAdjustScrollView = false): number { + const [viewportOffsetTop, setViewportOffsetTop] = useState(0); + const initialHeight = useRef(window.visualViewport?.height ?? window.innerHeight).current; + const cachedDefaultOffsetTop = useRef(0); + useEffect(() => { + const updateOffsetTop = (event?: Event) => { + let targetOffsetTop = window.visualViewport?.offsetTop || 0; + if (event?.target instanceof VisualViewport) { + targetOffsetTop = event.target.offsetTop; + } + + if (shouldAdjustScrollView && window.visualViewport) { + const adjustScrollY = Math.round(initialHeight - window.visualViewport.height); + if (cachedDefaultOffsetTop.current === 0) { + cachedDefaultOffsetTop.current = targetOffsetTop; + } + + if (adjustScrollY > targetOffsetTop) { + setViewportOffsetTop(adjustScrollY); + } else if (targetOffsetTop !== 0 && adjustScrollY === targetOffsetTop) { + setViewportOffsetTop(cachedDefaultOffsetTop.current); + } else { + setViewportOffsetTop(targetOffsetTop); + } + } else { + setViewportOffsetTop(targetOffsetTop); + } + }; + updateOffsetTop(); + const removeViewportResizeListener = addViewportResizeListener(updateOffsetTop); + + return () => { + removeViewportResizeListener(); + }; + }, [initialHeight, shouldAdjustScrollView]); + + useEffect(() => { + if (!shouldAdjustScrollView) { + return; + } + window.scrollTo({top: viewportOffsetTop}); + }, [shouldAdjustScrollView, viewportOffsetTop]); + + return viewportOffsetTop; +} diff --git a/src/libs/actions/Modal.ts b/src/libs/actions/Modal.ts index 6351d0165544..5f4f66760f17 100644 --- a/src/libs/actions/Modal.ts +++ b/src/libs/actions/Modal.ts @@ -60,6 +60,7 @@ function onModalDidClose() { onModalClose(); onModalClose = null; isNavigate = undefined; + setModalVisibility(false); } /** diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 242602b0654c..51d7c511aafb 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -21,14 +21,14 @@ import ScreenWrapper from '@components/ScreenWrapper'; import TaskHeaderActionButton from '@components/TaskHeaderActionButton'; import withCurrentReportID from '@components/withCurrentReportID'; import type {CurrentReportIDContextValue} from '@components/withCurrentReportID'; -import withViewportOffsetTop from '@components/withViewportOffsetTop'; -import type {ViewportOffsetTopProps} from '@components/withViewportOffsetTop'; import useAppFocusEvent from '@hooks/useAppFocusEvent'; import useLocalize from '@hooks/useLocalize'; import usePrevious from '@hooks/usePrevious'; import useThemeStyles from '@hooks/useThemeStyles'; +import useViewportOffsetTop from '@hooks/useViewportOffsetTop'; import useWindowDimensions from '@hooks/useWindowDimensions'; import Timing from '@libs/actions/Timing'; +import * as Browser from '@libs/Browser'; import Navigation from '@libs/Navigation/Navigation'; import clearReportNotifications from '@libs/Notification/clearReportNotifications'; import reportWithoutHasDraftSelector from '@libs/OnyxSelectors/reportWithoutHasDraftSelector'; @@ -47,6 +47,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; import type * as OnyxTypes from '@src/types/onyx'; +import type {Modal} from '@src/types/onyx'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import HeaderView from './HeaderView'; import ReportActionsView from './report/ReportActionsView'; @@ -55,6 +56,8 @@ import {ActionListContext, ReactionListContext} from './ReportScreenContext'; import type {ActionListContextType, ReactionListRef, ScrollPosition} from './ReportScreenContext'; type ReportScreenOnyxProps = { + /** Indicates if there is a modal currently visible or not */ + modal: OnyxEntry; /** Tells us if the sidebar has rendered */ isSidebarLoaded: OnyxEntry; @@ -93,7 +96,7 @@ type OnyxHOCProps = { type ReportScreenNavigationProps = StackScreenProps; -type ReportScreenProps = OnyxHOCProps & ViewportOffsetTopProps & CurrentReportIDContextValue & ReportScreenOnyxProps & ReportScreenNavigationProps; +type ReportScreenProps = OnyxHOCProps & CurrentReportIDContextValue & ReportScreenOnyxProps & ReportScreenNavigationProps; /** Get the currently viewed report ID as number */ function getReportID(route: ReportScreenNavigationProps['route']): string { @@ -131,7 +134,7 @@ function ReportScreen({ markReadyForHydration, policies = {}, isSidebarLoaded = false, - viewportOffsetTop, + modal = {isVisible: false}, isComposerFullSize = false, userLeavingStatus = false, currentReportID = '', @@ -257,6 +260,8 @@ function ReportScreen({ Timing.start(CONST.TIMING.CHAT_RENDER); Performance.markStart(CONST.TIMING.CHAT_RENDER); } + const [isComposerFocus, setIsComposerFocus] = useState(false); + const viewportOffsetTop = useViewportOffsetTop(Browser.isMobileSafari() && isComposerFocus && !modal?.isVisible); const {reportPendingAction, reportErrors} = ReportUtils.getReportOfflinePendingActionAndErrors(report); const screenWrapperStyle: ViewStyle[] = [styles.appContent, styles.flex1, {marginTop: viewportOffsetTop}]; @@ -638,6 +643,8 @@ function ReportScreen({ {isReportReadyForDisplay ? ( setIsComposerFocus(true)} + onComposerBlur={() => setIsComposerFocus(false)} report={report} pendingAction={reportPendingAction} isComposerFullSize={!!isComposerFullSize} @@ -657,81 +664,82 @@ function ReportScreen({ ReportScreen.displayName = 'ReportScreen'; -export default withViewportOffsetTop( - withCurrentReportID( - withOnyx( - { - isSidebarLoaded: { - key: ONYXKEYS.IS_SIDEBAR_LOADED, - }, - sortedAllReportActions: { - key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getReportID(route)}`, - canEvict: false, - selector: (allReportActions: OnyxEntry) => ReportActionsUtils.getSortedReportActionsForDisplay(allReportActions, true), - }, - report: { - key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${getReportID(route)}`, - allowStaleData: true, - selector: reportWithoutHasDraftSelector, - }, - reportMetadata: { - key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${getReportID(route)}`, - initialValue: { - isLoadingInitialReportActions: true, - isLoadingOlderReportActions: false, - isLoadingNewerReportActions: false, - }, - }, - isComposerFullSize: { - key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_IS_COMPOSER_FULL_SIZE}${getReportID(route)}`, - initialValue: false, - }, - betas: { - key: ONYXKEYS.BETAS, - }, - policies: { - key: ONYXKEYS.COLLECTION.POLICY, - allowStaleData: true, - }, - accountManagerReportID: { - key: ONYXKEYS.ACCOUNT_MANAGER_REPORT_ID, - initialValue: null, - }, - userLeavingStatus: { - key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_USER_IS_LEAVING_ROOM}${getReportID(route)}`, - initialValue: false, +export default withCurrentReportID( + withOnyx( + { + modal: { + key: ONYXKEYS.MODAL, + }, + isSidebarLoaded: { + key: ONYXKEYS.IS_SIDEBAR_LOADED, + }, + sortedAllReportActions: { + key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getReportID(route)}`, + canEvict: false, + selector: (allReportActions: OnyxEntry) => ReportActionsUtils.getSortedReportActionsForDisplay(allReportActions, true), + }, + report: { + key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${getReportID(route)}`, + allowStaleData: true, + selector: reportWithoutHasDraftSelector, + }, + reportMetadata: { + key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${getReportID(route)}`, + initialValue: { + isLoadingInitialReportActions: true, + isLoadingOlderReportActions: false, + isLoadingNewerReportActions: false, }, - parentReportAction: { - key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : 0}`, - selector: (parentReportActions: OnyxEntry, props: WithOnyxInstanceState): OnyxEntry => { - const parentReportActionID = props?.report?.parentReportActionID; - if (!parentReportActionID) { - return null; - } - return parentReportActions?.[parentReportActionID] ?? null; - }, - canEvict: false, + }, + isComposerFullSize: { + key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_IS_COMPOSER_FULL_SIZE}${getReportID(route)}`, + initialValue: false, + }, + betas: { + key: ONYXKEYS.BETAS, + }, + policies: { + key: ONYXKEYS.COLLECTION.POLICY, + allowStaleData: true, + }, + accountManagerReportID: { + key: ONYXKEYS.ACCOUNT_MANAGER_REPORT_ID, + initialValue: null, + }, + userLeavingStatus: { + key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_USER_IS_LEAVING_ROOM}${getReportID(route)}`, + initialValue: false, + }, + parentReportAction: { + key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : 0}`, + selector: (parentReportActions: OnyxEntry, props: WithOnyxInstanceState): OnyxEntry => { + const parentReportActionID = props?.report?.parentReportActionID; + if (!parentReportActionID) { + return null; + } + return parentReportActions?.[parentReportActionID] ?? null; }, + canEvict: false, }, - true, - )( - memo( - ReportScreen, - (prevProps, nextProps) => - prevProps.isSidebarLoaded === nextProps.isSidebarLoaded && - lodashIsEqual(prevProps.sortedAllReportActions, nextProps.sortedAllReportActions) && - lodashIsEqual(prevProps.reportMetadata, nextProps.reportMetadata) && - prevProps.isComposerFullSize === nextProps.isComposerFullSize && - lodashIsEqual(prevProps.betas, nextProps.betas) && - lodashIsEqual(prevProps.policies, nextProps.policies) && - prevProps.accountManagerReportID === nextProps.accountManagerReportID && - prevProps.userLeavingStatus === nextProps.userLeavingStatus && - prevProps.currentReportID === nextProps.currentReportID && - prevProps.viewportOffsetTop === nextProps.viewportOffsetTop && - lodashIsEqual(prevProps.parentReportAction, nextProps.parentReportAction) && - lodashIsEqual(prevProps.route, nextProps.route) && - lodashIsEqual(prevProps.report, nextProps.report), - ), + }, + true, + )( + memo( + ReportScreen, + (prevProps, nextProps) => + prevProps.isSidebarLoaded === nextProps.isSidebarLoaded && + lodashIsEqual(prevProps.sortedAllReportActions, nextProps.sortedAllReportActions) && + lodashIsEqual(prevProps.reportMetadata, nextProps.reportMetadata) && + prevProps.isComposerFullSize === nextProps.isComposerFullSize && + lodashIsEqual(prevProps.betas, nextProps.betas) && + lodashIsEqual(prevProps.policies, nextProps.policies) && + prevProps.accountManagerReportID === nextProps.accountManagerReportID && + prevProps.userLeavingStatus === nextProps.userLeavingStatus && + prevProps.currentReportID === nextProps.currentReportID && + lodashIsEqual(prevProps.modal, nextProps.modal) && + lodashIsEqual(prevProps.parentReportAction, nextProps.parentReportAction) && + lodashIsEqual(prevProps.route, nextProps.route) && + lodashIsEqual(prevProps.report, nextProps.report), ), ), ); diff --git a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx index 1e0e322be258..6face391dab3 100644 --- a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx +++ b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx @@ -61,6 +61,8 @@ type SuggestionsRef = { updateShouldShowSuggestionMenuToFalse: (shouldShowSuggestionMenu?: boolean) => void; setShouldBlockSuggestionCalc: (shouldBlock: boolean) => void; getSuggestions: () => Mention[] | Emoji[]; + onComposerFocus: () => void; + onComposerBlur: () => void; }; type ReportActionComposeOnyxProps = { @@ -85,6 +87,9 @@ type ReportActionComposeProps = ReportActionComposeOnyxProps & /** Whether the report is ready for display */ isReportReadyForDisplay?: boolean; + } & { + onComposerFocus: () => {}; + onComposerBlur: () => {}; }; // We want consistent auto focus behavior on input between native and mWeb so we have some auto focus management code that will @@ -107,6 +112,8 @@ function ReportActionCompose({ isReportReadyForDisplay = true, isEmptyChat, lastReportAction, + onComposerFocus, + onComposerBlur, }: ReportActionComposeProps) { const styles = useThemeStyles(); const {translate} = useLocalize(); @@ -296,6 +303,7 @@ function ReportActionCompose({ const onBlur = useCallback((event: NativeSyntheticEvent) => { const webEvent = event as unknown as FocusEvent; setIsFocused(false); + onComposerBlur(); if (suggestionsRef.current) { suggestionsRef.current.resetSuggestions(); } @@ -304,8 +312,14 @@ function ReportActionCompose({ } }, []); - const onFocus = useCallback(() => { + const handleFocus = useCallback(() => { setIsFocused(true); + onComposerFocus(); + }, []); + + const handleBlur = useCallback(() => { + setIsFocused(false); + onComposerBlur(); }, []); // resets the composer to normal size when @@ -436,8 +450,8 @@ function ReportActionCompose({ setIsCommentEmpty={setIsCommentEmpty} handleSendMessage={handleSendMessage} shouldShowComposeInput={shouldShowComposeInput} - onFocus={onFocus} - onBlur={onBlur} + onFocus={handleFocus} + onBlur={handleBlur} measureParentContainer={measureContainer} listHeight={listHeight} onValueChange={(value) => { diff --git a/src/pages/home/report/ReportFooter.tsx b/src/pages/home/report/ReportFooter.tsx index 3a787e1dbd0f..3acb254c7e46 100644 --- a/src/pages/home/report/ReportFooter.tsx +++ b/src/pages/home/report/ReportFooter.tsx @@ -51,6 +51,11 @@ type ReportFooterProps = ReportFooterOnyxProps & { /** Whether the composer is in full size */ isComposerFullSize?: boolean; + + onComposerFocus: () => void; + + /** A method to call when the input is blue */ + onComposerBlur: () => void; }; function ReportFooter({ @@ -63,6 +68,8 @@ function ReportFooter({ isReportReadyForDisplay = true, listHeight = 0, isComposerFullSize = false, + onComposerBlur, + onComposerFocus, }: ReportFooterProps) { const styles = useThemeStyles(); const {isOffline} = useNetwork(); @@ -137,6 +144,8 @@ function ReportFooter({ Date: Fri, 22 Mar 2024 00:06:54 +0700 Subject: [PATCH 2/9] update popover max height --- .../EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js | 2 +- src/styles/index.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js index 80521e04ddd9..0566f25159c4 100644 --- a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js +++ b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js @@ -24,7 +24,7 @@ const useEmojiPickerMenu = () => { const [preferredSkinTone] = usePreferredEmojiSkinTone(); const {windowHeight} = useWindowDimensions(true); const StyleUtils = useStyleUtils(); - const listStyle = StyleUtils.getEmojiPickerListHeight(isListFiltered, windowHeight * 0.95); + const listStyle = StyleUtils.getEmojiPickerListHeight(isListFiltered, windowHeight); useEffect(() => { setFilteredEmojis(allEmojis); diff --git a/src/styles/index.ts b/src/styles/index.ts index b0c88d151484..26fe60ad61b2 100644 --- a/src/styles/index.ts +++ b/src/styles/index.ts @@ -1639,7 +1639,7 @@ const styles = (theme: ThemeColors) => popoverInnerContainer: { paddingTop: 0, // adjusting this because the mobile modal adds additional padding that we don't need for our layout - maxHeight: '95%', + maxHeight: '100%', }, menuItemTextContainer: { From 4df3eb8345ec66c70730588f2e74c265f0c73e9b Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Fri, 22 Mar 2024 00:37:19 +0700 Subject: [PATCH 3/9] fix type check --- .../ReportActionCompose/ReportActionCompose.tsx | 11 ++++++----- src/pages/home/report/ReportFooter.tsx | 3 ++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx index 6face391dab3..4b1ec41a498e 100644 --- a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx +++ b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx @@ -61,8 +61,6 @@ type SuggestionsRef = { updateShouldShowSuggestionMenuToFalse: (shouldShowSuggestionMenu?: boolean) => void; setShouldBlockSuggestionCalc: (shouldBlock: boolean) => void; getSuggestions: () => Mention[] | Emoji[]; - onComposerFocus: () => void; - onComposerBlur: () => void; }; type ReportActionComposeOnyxProps = { @@ -87,9 +85,12 @@ type ReportActionComposeProps = ReportActionComposeOnyxProps & /** Whether the report is ready for display */ isReportReadyForDisplay?: boolean; - } & { - onComposerFocus: () => {}; - onComposerBlur: () => {}; + + /** A method to call when the input is focus */ + onComposerFocus: () => void; + + /** A method to call when the input is blur */ + onComposerBlur: () => void; }; // We want consistent auto focus behavior on input between native and mWeb so we have some auto focus management code that will diff --git a/src/pages/home/report/ReportFooter.tsx b/src/pages/home/report/ReportFooter.tsx index 3acb254c7e46..bd143f9ef196 100644 --- a/src/pages/home/report/ReportFooter.tsx +++ b/src/pages/home/report/ReportFooter.tsx @@ -52,9 +52,10 @@ type ReportFooterProps = ReportFooterOnyxProps & { /** Whether the composer is in full size */ isComposerFullSize?: boolean; + /** A method to call when the input is focus */ onComposerFocus: () => void; - /** A method to call when the input is blue */ + /** A method to call when the input is blur */ onComposerBlur: () => void; }; From 52e658543be8ac6b6ba1f35764dc5ef85e50bc19 Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Fri, 22 Mar 2024 00:39:21 +0700 Subject: [PATCH 4/9] Revert "update popover max height" This reverts commit 750d321749519ed0e14306739be3ef109d126211. --- .../EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js | 2 +- src/styles/index.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js index 0566f25159c4..80521e04ddd9 100644 --- a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js +++ b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js @@ -24,7 +24,7 @@ const useEmojiPickerMenu = () => { const [preferredSkinTone] = usePreferredEmojiSkinTone(); const {windowHeight} = useWindowDimensions(true); const StyleUtils = useStyleUtils(); - const listStyle = StyleUtils.getEmojiPickerListHeight(isListFiltered, windowHeight); + const listStyle = StyleUtils.getEmojiPickerListHeight(isListFiltered, windowHeight * 0.95); useEffect(() => { setFilteredEmojis(allEmojis); diff --git a/src/styles/index.ts b/src/styles/index.ts index 26fe60ad61b2..b0c88d151484 100644 --- a/src/styles/index.ts +++ b/src/styles/index.ts @@ -1639,7 +1639,7 @@ const styles = (theme: ThemeColors) => popoverInnerContainer: { paddingTop: 0, // adjusting this because the mobile modal adds additional padding that we don't need for our layout - maxHeight: '100%', + maxHeight: '95%', }, menuItemTextContainer: { From 14232fe33bece6ec0e627ace54d097e8a3f1c6c9 Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Fri, 22 Mar 2024 00:56:06 +0700 Subject: [PATCH 5/9] update code review and fix eslint --- src/components/EmojiPicker/EmojiPicker.js | 3 +- src/hooks/useViewportOffsetTop/index.ts | 2 +- src/libs/actions/Modal.ts | 14 ++++---- src/pages/home/ReportScreen.tsx | 3 +- .../ReportActionCompose.tsx | 34 +++++++++---------- 5 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPicker.js b/src/components/EmojiPicker/EmojiPicker.js index fd0fe786b632..0d6143416440 100644 --- a/src/components/EmojiPicker/EmojiPicker.js +++ b/src/components/EmojiPicker/EmojiPicker.js @@ -176,7 +176,8 @@ const EmojiPicker = forwardRef((props, ref) => { onModalShow={focusEmojiSearchInput} onModalHide={onModalHide.current} hideModalContentWhileAnimating - shouldSetModalVisibility={Browser.isMobile()} + // shouldSetModalVisibility is true for mobile safari to handle adjust virtual viewport position when toggle modal visibility + shouldSetModalVisibility={Browser.isMobileSafari()} animationInTiming={1} animationOutTiming={1} anchorPosition={{ diff --git a/src/hooks/useViewportOffsetTop/index.ts b/src/hooks/useViewportOffsetTop/index.ts index 7bb13f55c5ee..55b0345d01ff 100644 --- a/src/hooks/useViewportOffsetTop/index.ts +++ b/src/hooks/useViewportOffsetTop/index.ts @@ -10,7 +10,7 @@ export default function useViewportOffsetTop(shouldAdjustScrollView = false): nu const cachedDefaultOffsetTop = useRef(0); useEffect(() => { const updateOffsetTop = (event?: Event) => { - let targetOffsetTop = window.visualViewport?.offsetTop || 0; + let targetOffsetTop = window.visualViewport?.offsetTop ?? 0; if (event?.target instanceof VisualViewport) { targetOffsetTop = event.target.offsetTop; } diff --git a/src/libs/actions/Modal.ts b/src/libs/actions/Modal.ts index 5f4f66760f17..928b080b50f8 100644 --- a/src/libs/actions/Modal.ts +++ b/src/libs/actions/Modal.ts @@ -49,6 +49,13 @@ function close(onModalCloseCallback: () => void, isNavigating = true) { closeTop(); } +/** + * Allows other parts of the app to know when a modal has been opened or closed + */ +function setModalVisibility(isVisible: boolean) { + Onyx.merge(ONYXKEYS.MODAL, {isVisible}); +} + function onModalDidClose() { if (!onModalClose) { return; @@ -63,13 +70,6 @@ function onModalDidClose() { setModalVisibility(false); } -/** - * Allows other parts of the app to know when a modal has been opened or closed - */ -function setModalVisibility(isVisible: boolean) { - Onyx.merge(ONYXKEYS.MODAL, {isVisible}); -} - /** * Allows other parts of app to know that an alert modal is about to open. * This will trigger as soon as a modal is opened but not yet visible while animation is running. diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 51d7c511aafb..4c23cf6be6c1 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -47,7 +47,6 @@ import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; import type * as OnyxTypes from '@src/types/onyx'; -import type {Modal} from '@src/types/onyx'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import HeaderView from './HeaderView'; import ReportActionsView from './report/ReportActionsView'; @@ -57,7 +56,7 @@ import type {ActionListContextType, ReactionListRef, ScrollPosition} from './Rep type ReportScreenOnyxProps = { /** Indicates if there is a modal currently visible or not */ - modal: OnyxEntry; + modal: OnyxEntry; /** Tells us if the sidebar has rendered */ isSidebarLoaded: OnyxEntry; diff --git a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx index 4b1ec41a498e..428e09d09fc7 100644 --- a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx +++ b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx @@ -301,27 +301,25 @@ function ReportActionCompose({ isKeyboardVisibleWhenShowingModalRef.current = true; }, []); - const onBlur = useCallback((event: NativeSyntheticEvent) => { - const webEvent = event as unknown as FocusEvent; - setIsFocused(false); - onComposerBlur(); - if (suggestionsRef.current) { - suggestionsRef.current.resetSuggestions(); - } - if (webEvent.relatedTarget && webEvent.relatedTarget === actionButtonRef.current) { - isKeyboardVisibleWhenShowingModalRef.current = true; - } - }, []); + const onBlur = useCallback( + (event: NativeSyntheticEvent) => { + const webEvent = event as unknown as FocusEvent; + setIsFocused(false); + onComposerBlur(); + if (suggestionsRef.current) { + suggestionsRef.current.resetSuggestions(); + } + if (webEvent.relatedTarget && webEvent.relatedTarget === actionButtonRef.current) { + isKeyboardVisibleWhenShowingModalRef.current = true; + } + }, + [onComposerBlur], + ); const handleFocus = useCallback(() => { setIsFocused(true); onComposerFocus(); - }, []); - - const handleBlur = useCallback(() => { - setIsFocused(false); - onComposerBlur(); - }, []); + }, [onComposerFocus]); // resets the composer to normal size when // the send button is pressed. @@ -452,7 +450,7 @@ function ReportActionCompose({ handleSendMessage={handleSendMessage} shouldShowComposeInput={shouldShowComposeInput} onFocus={handleFocus} - onBlur={handleBlur} + onBlur={onBlur} measureParentContainer={measureContainer} listHeight={listHeight} onValueChange={(value) => { From 967184b5410282951a05208da76889342099ff6d Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Tue, 26 Mar 2024 08:54:28 +0700 Subject: [PATCH 6/9] update condition avoid regression --- src/components/EmojiPicker/EmojiPicker.js | 3 +-- .../EmojiPickerMenu/useEmojiPickerMenu.js | 1 + src/libs/actions/Modal.ts | 15 +++++++-------- src/pages/home/ReportScreen.tsx | 6 +++--- .../ReportActionCompose/ReportActionCompose.tsx | 12 ++++++------ 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPicker.js b/src/components/EmojiPicker/EmojiPicker.js index 0d6143416440..6bceaf570ccc 100644 --- a/src/components/EmojiPicker/EmojiPicker.js +++ b/src/components/EmojiPicker/EmojiPicker.js @@ -176,8 +176,7 @@ const EmojiPicker = forwardRef((props, ref) => { onModalShow={focusEmojiSearchInput} onModalHide={onModalHide.current} hideModalContentWhileAnimating - // shouldSetModalVisibility is true for mobile safari to handle adjust virtual viewport position when toggle modal visibility - shouldSetModalVisibility={Browser.isMobileSafari()} + shouldSetModalVisibility={false} animationInTiming={1} animationOutTiming={1} anchorPosition={{ diff --git a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js index 80521e04ddd9..86cb6d921c36 100644 --- a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js +++ b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js @@ -24,6 +24,7 @@ const useEmojiPickerMenu = () => { const [preferredSkinTone] = usePreferredEmojiSkinTone(); const {windowHeight} = useWindowDimensions(true); const StyleUtils = useStyleUtils(); + // calculate the height of the emoji picker based popoverInnerContainer style has maxHeight is 95% const listStyle = StyleUtils.getEmojiPickerListHeight(isListFiltered, windowHeight * 0.95); useEffect(() => { diff --git a/src/libs/actions/Modal.ts b/src/libs/actions/Modal.ts index 928b080b50f8..6351d0165544 100644 --- a/src/libs/actions/Modal.ts +++ b/src/libs/actions/Modal.ts @@ -49,13 +49,6 @@ function close(onModalCloseCallback: () => void, isNavigating = true) { closeTop(); } -/** - * Allows other parts of the app to know when a modal has been opened or closed - */ -function setModalVisibility(isVisible: boolean) { - Onyx.merge(ONYXKEYS.MODAL, {isVisible}); -} - function onModalDidClose() { if (!onModalClose) { return; @@ -67,7 +60,13 @@ function onModalDidClose() { onModalClose(); onModalClose = null; isNavigate = undefined; - setModalVisibility(false); +} + +/** + * Allows other parts of the app to know when a modal has been opened or closed + */ +function setModalVisibility(isVisible: boolean) { + Onyx.merge(ONYXKEYS.MODAL, {isVisible}); } /** diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 354f7890ed50..366e06d0b414 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -55,7 +55,7 @@ import {ActionListContext, ReactionListContext} from './ReportScreenContext'; import type {ActionListContextType, ReactionListRef, ScrollPosition} from './ReportScreenContext'; type ReportScreenOnyxProps = { - /** Indicates if there is a modal currently visible or not */ + /** Get modal status */ modal: OnyxEntry; /** Tells us if the sidebar has rendered */ isSidebarLoaded: OnyxEntry; @@ -133,7 +133,7 @@ function ReportScreen({ markReadyForHydration, policies = {}, isSidebarLoaded = false, - modal = {isVisible: false}, + modal, isComposerFullSize = false, userLeavingStatus = false, currentReportID = '', @@ -262,7 +262,7 @@ function ReportScreen({ Performance.markStart(CONST.TIMING.CHAT_RENDER); } const [isComposerFocus, setIsComposerFocus] = useState(false); - const viewportOffsetTop = useViewportOffsetTop(Browser.isMobileSafari() && isComposerFocus && !modal?.isVisible); + const viewportOffsetTop = useViewportOffsetTop(Browser.isMobileSafari() && isComposerFocus && !modal?.willAlertModalBecomeVisible); const {reportPendingAction, reportErrors} = ReportUtils.getReportOfflinePendingActionAndErrors(report); const screenWrapperStyle: ViewStyle[] = [styles.appContent, styles.flex1, {marginTop: viewportOffsetTop}]; diff --git a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx index 428e09d09fc7..70340e9e1fec 100644 --- a/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx +++ b/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx @@ -87,10 +87,10 @@ type ReportActionComposeProps = ReportActionComposeOnyxProps & isReportReadyForDisplay?: boolean; /** A method to call when the input is focus */ - onComposerFocus: () => void; + onComposerFocus?: () => void; /** A method to call when the input is blur */ - onComposerBlur: () => void; + onComposerBlur?: () => void; }; // We want consistent auto focus behavior on input between native and mWeb so we have some auto focus management code that will @@ -305,7 +305,7 @@ function ReportActionCompose({ (event: NativeSyntheticEvent) => { const webEvent = event as unknown as FocusEvent; setIsFocused(false); - onComposerBlur(); + onComposerBlur?.(); if (suggestionsRef.current) { suggestionsRef.current.resetSuggestions(); } @@ -316,9 +316,9 @@ function ReportActionCompose({ [onComposerBlur], ); - const handleFocus = useCallback(() => { + const onFocus = useCallback(() => { setIsFocused(true); - onComposerFocus(); + onComposerFocus?.(); }, [onComposerFocus]); // resets the composer to normal size when @@ -449,7 +449,7 @@ function ReportActionCompose({ setIsCommentEmpty={setIsCommentEmpty} handleSendMessage={handleSendMessage} shouldShowComposeInput={shouldShowComposeInput} - onFocus={handleFocus} + onFocus={onFocus} onBlur={onBlur} measureParentContainer={measureContainer} listHeight={listHeight} From 8114339be16e52e43662adddadf37ebd36302c66 Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Tue, 26 Mar 2024 09:24:44 +0700 Subject: [PATCH 7/9] update code review Co-authored-by: Situ Chandra Shil <108292595+situchan@users.noreply.github.com> --- src/pages/home/ReportScreen.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 366e06d0b414..e4a20aa45989 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -57,6 +57,7 @@ import type {ActionListContextType, ReactionListRef, ScrollPosition} from './Rep type ReportScreenOnyxProps = { /** Get modal status */ modal: OnyxEntry; + /** Tells us if the sidebar has rendered */ isSidebarLoaded: OnyxEntry; From 5f3f51c29fb2064d2e5ee7b1292aae613e27c28c Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Tue, 26 Mar 2024 10:17:17 +0700 Subject: [PATCH 8/9] revert emoji picker using cached height --- .../EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js index 86cb6d921c36..ba3001703c04 100644 --- a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js +++ b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js @@ -22,7 +22,7 @@ const useEmojiPickerMenu = () => { const isListFiltered = allEmojis.length !== filteredEmojis.length; const {preferredLocale} = useLocalize(); const [preferredSkinTone] = usePreferredEmojiSkinTone(); - const {windowHeight} = useWindowDimensions(true); + const {windowHeight} = useWindowDimensions(); const StyleUtils = useStyleUtils(); // calculate the height of the emoji picker based popoverInnerContainer style has maxHeight is 95% const listStyle = StyleUtils.getEmojiPickerListHeight(isListFiltered, windowHeight * 0.95); From 1f8265cfd4be404e2af9732c9bf19cc19c868597 Mon Sep 17 00:00:00 2001 From: Cong Pham Date: Wed, 27 Mar 2024 01:56:15 +0700 Subject: [PATCH 9/9] cleanup function and update comment --- .../EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js | 6 +++++- src/hooks/useViewportOffsetTop/index.ts | 6 +----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js index ba3001703c04..c6f9f601f4df 100644 --- a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js +++ b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js @@ -24,7 +24,11 @@ const useEmojiPickerMenu = () => { const [preferredSkinTone] = usePreferredEmojiSkinTone(); const {windowHeight} = useWindowDimensions(); const StyleUtils = useStyleUtils(); - // calculate the height of the emoji picker based popoverInnerContainer style has maxHeight is 95% + /** + * At EmojiPicker has set innerContainerStyle with maxHeight: '95%' by styles.popoverInnerContainer + * to avoid the list style to be cut off due to the list height being larger than the container height + * so we need to calculate listStyle based on the height of the window and innerContainerStyle at the EmojiPicker + */ const listStyle = StyleUtils.getEmojiPickerListHeight(isListFiltered, windowHeight * 0.95); useEffect(() => { diff --git a/src/hooks/useViewportOffsetTop/index.ts b/src/hooks/useViewportOffsetTop/index.ts index 55b0345d01ff..56fb19187c4f 100644 --- a/src/hooks/useViewportOffsetTop/index.ts +++ b/src/hooks/useViewportOffsetTop/index.ts @@ -33,11 +33,7 @@ export default function useViewportOffsetTop(shouldAdjustScrollView = false): nu } }; updateOffsetTop(); - const removeViewportResizeListener = addViewportResizeListener(updateOffsetTop); - - return () => { - removeViewportResizeListener(); - }; + return addViewportResizeListener(updateOffsetTop); }, [initialHeight, shouldAdjustScrollView]); useEffect(() => {