Improve Side Pane behavior across various Modals and navigation actions #58576
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,79 @@ | ||
| import React, {useEffect} from 'react'; | ||
| // eslint-disable-next-line no-restricted-imports | ||
| import {Animated, View} from 'react-native'; | ||
| import {useOnyx} from 'react-native-onyx'; | ||
| // @ts-expect-error This is a workaround to display HelpPane on top of everything, | ||
|
Contributor
Author
There was a problem hiding this comment. I’ve really tried every possible approach 😮💨
|
||
| // Modal from react-native can't be used here, as it would block interactions with the rest of the app | ||
| import ModalPortal from 'react-native-web/dist/exports/Modal/ModalPortal'; | ||
| import FocusTrapForModal from '@components/FocusTrap/FocusTrapForModal'; | ||
| import SidePaneOverlay from '@components/SidePane/SidePaneOverlay'; | ||
| import useKeyboardShortcut from '@hooks/useKeyboardShortcut'; | ||
| import useResponsiveLayout from '@hooks/useResponsiveLayout'; | ||
| import useSafeAreaPaddings from '@hooks/useSafeAreaPaddings'; | ||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
| import CONST from '@src/CONST'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import HelpContent from './HelpContent'; | ||
| import type HelpProps from './types'; | ||
|
|
||
| function Help({sidePaneTranslateX, closeSidePane, shouldHideSidePaneBackdrop}: HelpProps) { | ||
| const styles = useThemeStyles(); | ||
| const {isExtraLargeScreenWidth, shouldUseNarrowLayout} = useResponsiveLayout(); | ||
| const {paddingTop, paddingBottom} = useSafeAreaPaddings(); | ||
| const [isRHPVisible = false] = useOnyx(ONYXKEYS.MODAL, {selector: (modal) => modal?.type === CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED}); | ||
|
Contributor
Author
There was a problem hiding this comment. This logic is necessary to prevent showing overlay twice: Screen.Recording.2025-03-18.at.14.29.44.mov |
||
|
|
||
| const onCloseSidePaneOnSmallScreens = () => { | ||
| if (isExtraLargeScreenWidth) { | ||
| return; | ||
| } | ||
|
|
||
| closeSidePane(); | ||
| }; | ||
|
|
||
| // Close side pane on escape key press | ||
| useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ESCAPE, () => closeSidePane(), {isActive: !isExtraLargeScreenWidth, shouldBubble: false}); | ||
|
Comment on lines
+33
to
+34
Contributor
There was a problem hiding this comment. I think I found an odd behavior while testing this. After closing the pane on a narrow screen (with the escape key or the < arrow) and then expanding to a wide screen view, the modal auto-reopens for some reason. It's also happening on main so NAB. Screen.Recording.2025-03-19.at.4.58.44.PM.mov
Contributor
Author
There was a problem hiding this comment. @francoisl @brunovjk This is the intended way it should work. When transitioning from a wide to a narrow screen, the pane is closed to prevent it from covering the rest of the app. When switching back to a wide screen, the previous state is preserved. If the pane was open before on a large screen, it will be shown again to the user. That is why there are two booleans in the NVP: type SidePane = {
/** Whether the side pane is open on large screens */
open: boolean;
/** Whether the side pane is open on small screens */
openNarrowScreen: boolean;
}; |
||
|
|
||
| // Close side pane on small screens when navigation keyboard shortcuts are used | ||
| useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.SEARCH, onCloseSidePaneOnSmallScreens, {shouldBubble: true}); | ||
| useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.NEW_CHAT, onCloseSidePaneOnSmallScreens, {shouldBubble: true}); | ||
| useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.SHORTCUTS, onCloseSidePaneOnSmallScreens, {shouldBubble: true}); | ||
|
Comment on lines
+36
to
+39
Contributor
Author
There was a problem hiding this comment. Logic for hiding the side pane when user triggers navigation shortcuts: Screen.Recording.2025-03-18.at.13.48.09.mov |
||
|
|
||
| // Web back button: push history state and close side pane on popstate | ||
| useEffect(() => { | ||
| window.history.pushState({isSidePaneOpen: true}, '', null); | ||
| const handlePopState = () => { | ||
| if (isExtraLargeScreenWidth) { | ||
| return; | ||
| } | ||
|
|
||
| closeSidePane(); | ||
| }; | ||
|
|
||
| window.addEventListener('popstate', handlePopState); | ||
| return () => window.removeEventListener('popstate', handlePopState); | ||
| // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps | ||
| }, []); | ||
|
Comment on lines
+42
to
+55
Contributor
Author
There was a problem hiding this comment. This part handles the back button behavior on the web. We use a similar approach for Modals/Popovers. It's not perfect, but it works in most cases: Screen.Recording.2025-03-18.at.14.35.24.movOne known bug that I won't be able to fix is that when opening both a Modal ( Screen.Recording.2025-03-18.at.14.38.34.mov |
||
|
|
||
| return ( | ||
| <ModalPortal> | ||
| <FocusTrapForModal active> | ||
| <View style={styles.sidePaneContainer}> | ||
| <View> | ||
| {!shouldHideSidePaneBackdrop && ( | ||
| <SidePaneOverlay | ||
| onBackdropPress={closeSidePane} | ||
| isRHPVisible={isRHPVisible} | ||
| /> | ||
| )} | ||
| </View> | ||
| <Animated.View | ||
| style={[styles.sidePaneContent(shouldUseNarrowLayout, isExtraLargeScreenWidth), {transform: [{translateX: sidePaneTranslateX.current}], paddingTop, paddingBottom}]} | ||
| > | ||
| <HelpContent closeSidePane={closeSidePane} /> | ||
| </Animated.View> | ||
| </View> | ||
| </FocusTrapForModal> | ||
| </ModalPortal> | ||
| ); | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This was a mistake that was in the code for couple months 😅