-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[navigation-refactor] Style based approach for the three pane layout #22437
Merged
mountiny
merged 44 commits into
Expensify:main
from
adamgrzybowski:@swm/style-based-three-pane-view
Aug 30, 2023
Merged
Changes from all commits
Commits
Show all changes
44 commits
Select commit
Hold shift + click to select a range
89305e4
switch to style based three pane view
adamgrzybowski b0bdde7
add temporary patch to never detach HomeScreen
adamgrzybowski c7e411b
add prettier adjustments
adamgrzybowski d015ad5
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 82652c9
Merge branch 'main' into @swm/style-based-three-pane-view
WoLewicki 35a1a5d
update comments
adamgrzybowski e166c92
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 36914d5
Update patches/@react-navigation+stack+6.3.16.patch
adamgrzybowski 17248f5
change name for style object
adamgrzybowski c63f5f8
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 25b3dc6
fix patch
adamgrzybowski 2cc49e4
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski a897f62
improve patch for stack
adamgrzybowski 59650dc
use better syntax
adamgrzybowski 8424344
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 97ae2e0
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 7d66ad6
add adjustments
adamgrzybowski c312479
move styles to style file
adamgrzybowski c48d814
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 2b6f70b
fix prettier
adamgrzybowski 27da40c
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski f3bb2f2
adjust styling
adamgrzybowski b17c78f
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 59ec44c
fix patch conflicts v2
adamgrzybowski e16e8f8
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski ddf015e
fix problem with safari
adamgrzybowski ab49778
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski dbece00
add NoDropZone for RigtModalNavigator
adamgrzybowski 4a2de13
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski af907c5
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski f87274a
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 09fa219
adjustment for position fixed
adamgrzybowski ad6d7aa
make styles for navigation modal card the same on desktop and web
adamgrzybowski c3d2154
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski e0a54ab
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski c1b0919
adjust screen in auth screens
adamgrzybowski ae29512
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 76199a0
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 450aa71
split patch
adamgrzybowski a6041d8
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 830da58
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski baaf9ab
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski 6c823b2
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski ec372f7
Merge branch 'main' into @swm/style-based-three-pane-view
adamgrzybowski File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
File renamed without changes.
68 changes: 68 additions & 0 deletions
68
patches/@react-navigation+stack+6.3.16+002+dontDetachScreen.patch
This file contains 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 |
---|---|---|
@@ -0,0 +1,68 @@ | ||
diff --git a/node_modules/@react-navigation/stack/lib/module/views/Stack/Card.js b/node_modules/@react-navigation/stack/lib/module/views/Stack/Card.js | ||
index 4bedb81..155d87f 100644 | ||
--- a/node_modules/@react-navigation/stack/lib/module/views/Stack/Card.js | ||
+++ b/node_modules/@react-navigation/stack/lib/module/views/Stack/Card.js | ||
@@ -123,7 +123,7 @@ export default class Card extends React.Component { | ||
animation(gesture, { | ||
...spec.config, | ||
// Detecting if the user used swipe gesture on iOS safari to trigger navigation in the browser history. | ||
- duration: getIsEdgeDragGesture() ? 0 : undefined, | ||
+ duration: getIsEdgeDragGesture() ? 0 : spec.config.duration, | ||
velocity, | ||
toValue, | ||
useNativeDriver, | ||
diff --git a/node_modules/@react-navigation/stack/lib/module/views/Stack/CardContainer.js b/node_modules/@react-navigation/stack/lib/module/views/Stack/CardContainer.js | ||
index b595af8..870be65 100644 | ||
--- a/node_modules/@react-navigation/stack/lib/module/views/Stack/CardContainer.js | ||
+++ b/node_modules/@react-navigation/stack/lib/module/views/Stack/CardContainer.js | ||
@@ -1,7 +1,7 @@ | ||
import { getHeaderTitle, HeaderBackContext, HeaderHeightContext, HeaderShownContext } from '@react-navigation/elements'; | ||
import { useTheme } from '@react-navigation/native'; | ||
import * as React from 'react'; | ||
-import { StyleSheet, View } from 'react-native'; | ||
+import { Platform, StyleSheet, View } from 'react-native'; | ||
import ModalPresentationContext from '../../utils/ModalPresentationContext'; | ||
import useKeyboardManager from '../../utils/useKeyboardManager'; | ||
import Card from './Card'; | ||
@@ -215,7 +215,8 @@ function CardContainer(_ref) { | ||
display: | ||
// Hide unfocused screens when animation isn't enabled | ||
// This is also necessary for a11y on web | ||
- animationEnabled === false && isNextScreenTransparent === false && detachCurrentScreen !== false && !focused ? 'none' : 'flex' | ||
+ animationEnabled === false && isNextScreenTransparent === false && detachCurrentScreen !== false && !focused ? 'none' : 'flex', | ||
+ zIndex: Platform.OS === 'web' ? 'auto' : undefined, | ||
}, StyleSheet.absoluteFill] | ||
}, /*#__PURE__*/React.createElement(View, { | ||
style: styles.container | ||
diff --git a/node_modules/@react-navigation/stack/lib/module/views/Stack/CardStack.js b/node_modules/@react-navigation/stack/lib/module/views/Stack/CardStack.js | ||
index 7558eb3..b7bb75e 100644 | ||
--- a/node_modules/@react-navigation/stack/lib/module/views/Stack/CardStack.js | ||
+++ b/node_modules/@react-navigation/stack/lib/module/views/Stack/CardStack.js | ||
@@ -356,6 +356,9 @@ export default class CardStack extends React.Component { | ||
extrapolate: 'clamp' | ||
}) : STATE_TRANSITIONING_OR_BELOW_TOP; | ||
} | ||
+ | ||
+ const isHomeScreenAndNotOnTop = route.name === 'Home' && isScreenActive !== STATE_ON_TOP; | ||
+ | ||
const { | ||
headerShown = true, | ||
headerTransparent, | ||
@@ -389,7 +392,7 @@ export default class CardStack extends React.Component { | ||
key: route.key, | ||
style: StyleSheet.absoluteFill, | ||
enabled: detachInactiveScreens, | ||
- active: isScreenActive, | ||
+ active: isHomeScreenAndNotOnTop ? STATE_TRANSITIONING_OR_BELOW_TOP : isScreenActive, | ||
freezeOnBlur: freezeOnBlur, | ||
pointerEvents: "box-none" | ||
}, /*#__PURE__*/React.createElement(CardContainer, { | ||
@@ -423,7 +426,7 @@ export default class CardStack extends React.Component { | ||
onTransitionStart: onTransitionStart, | ||
onTransitionEnd: onTransitionEnd, | ||
isNextScreenTransparent: isNextScreenTransparent, | ||
- detachCurrentScreen: detachCurrentScreen | ||
+ detachCurrentScreen: isHomeScreenAndNotOnTop ? false : detachCurrentScreen, | ||
})); | ||
})), isFloatHeaderAbsolute ? floatingHeader : null); | ||
} |
This file contains 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 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
21 changes: 0 additions & 21 deletions
21
src/libs/Navigation/AppNavigator/Navigators/FullScreenNavigator.js
This file was deleted.
Oops, something went wrong.
This file contains 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 |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import React from 'react'; | ||
import {Animated, View} from 'react-native'; | ||
import {useCardAnimation} from '@react-navigation/stack'; | ||
|
||
import PropTypes from 'prop-types'; | ||
import styles from '../../../../styles/styles'; | ||
|
||
import PressableWithoutFeedback from '../../../../components/Pressable/PressableWithoutFeedback'; | ||
import useLocalize from '../../../../hooks/useLocalize'; | ||
import CONST from '../../../../CONST'; | ||
|
||
const propTypes = { | ||
/* Callback to close the modal */ | ||
onPress: PropTypes.func.isRequired, | ||
}; | ||
|
||
function Overlay(props) { | ||
const {current} = useCardAnimation(); | ||
const {translate} = useLocalize(); | ||
|
||
return ( | ||
<Animated.View style={styles.overlayStyles(current)}> | ||
<View style={[styles.flex1, styles.flexColumn]}> | ||
{/* In the latest Electron version buttons can't be both clickable and draggable. | ||
That's why we added this workaround. Because of two Pressable components on the desktop app | ||
we have 30px draggable ba at the top and the rest of the dimmed area is clickable. On other devices, | ||
everything behaves normally like one big pressable */} | ||
<PressableWithoutFeedback | ||
style={[styles.draggableTopBar]} | ||
onPress={props.onPress} | ||
accessibilityLabel={translate('common.close')} | ||
accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON} | ||
/> | ||
<PressableWithoutFeedback | ||
style={[styles.flex1]} | ||
onPress={props.onPress} | ||
accessibilityLabel={translate('common.close')} | ||
accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON} | ||
noDragArea | ||
/> | ||
</View> | ||
</Animated.View> | ||
); | ||
} | ||
|
||
Overlay.propTypes = propTypes; | ||
Overlay.displayName = 'Overlay'; | ||
|
||
export default Overlay; |
Oops, something went wrong.
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.
Why are we pushing the whole navigation to the right with marginLeft?
Then later reset it with transform on screens.
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.
cc: @allroundexperts @mananjadhav @adamgrzybowski @aimane-chnaif
I am looking into this issue #31573. We do have a solution #31573 (comment). But I am trying to understand this better.