-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[navigation-refactor] Style based approach for the three pane layout #22437
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
Changes from all commits
89305e4
b0bdde7
c7e411b
d015ad5
82652c9
35a1a5d
e166c92
36914d5
17248f5
c63f5f8
25b3dc6
2cc49e4
a897f62
59650dc
8424344
97ae2e0
7d66ad6
c312479
c48d814
2b6f70b
27da40c
f3bb2f2
b17c78f
59ec44c
e16e8f8
ddf015e
ab49778
dbece00
4a2de13
af907c5
f87274a
09fa219
ad6d7aa
c3d2154
e0a54ab
c1b0919
ae29512
76199a0
450aa71
a6041d8
830da58
baaf9ab
6c823b2
ec372f7
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 |
|---|---|---|
| @@ -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 was deleted.
| 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} | ||
| /> | ||
|
Comment on lines
+24
to
+33
Contributor
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. @adamgrzybowski As desktop app is deprecated, Is it fine to remove this workaround completely?
Contributor
Author
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. Seems like it should be safe |
||
| <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; | ||
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.
Uh oh!
There was an error while loading. Please reload this page.
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.