-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: web compatibility #406
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 3 commits
94670f3
96c8515
3867bfa
a9cd8fb
97236fd
ee685df
cd9bc75
c62c1ee
b8bcc54
f508722
107f5d7
0dd754a
59d1586
a586cea
b4343e0
f6bd3a9
9fd9071
c74b441
4b485d4
5cbb2c9
251f13d
ba0cdfc
19bb6b2
831c3aa
81019e2
172f8df
9d6dc77
362b23b
868ed93
23af89f
9304881
945c296
376b5b4
4f568bc
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,3 @@ | ||
| import AnimatedEvent from 'react-native/Libraries/Animated/src/AnimatedEvent'; | ||
|
|
||
| export default AnimatedEvent; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| import AnimatedEvent from 'react-native-web/dist/vendor/react-native/Animated/AnimatedEvent'; | ||
|
|
||
| export default AnimatedEvent; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| import React from 'react'; | ||
| import { | ||
| ScrollView, | ||
| Slider, | ||
| Switch, | ||
| TextInput, | ||
| ToolbarAndroid, | ||
| ViewPagerAndroid, | ||
| DrawerLayoutAndroid, | ||
| WebView, | ||
|
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. This module is being removed from React Native and isn't included in
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. Which one in detail? Currently, I see that all of these components are exported though some of them as 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.
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. Thanks for the info! |
||
| FlatList, | ||
| TouchableWithoutFeedback, | ||
| findNodeHandle, | ||
| } from 'react-native'; | ||
| import gestureHandlerRootHOC from './gestureHandlerRootHOC'; | ||
|
|
||
| const State = { | ||
| UNDETERMINED: 'UNDETERMINED', | ||
|
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. Can we move it out somewhere?
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. What should we move out in detail? State constants?
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. Yes. If you need them here, I wish to have them in separated file in order not to duplicate code.
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. Sure, makes more than sense 😊 Was not sure before since these constants were used in
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. Oh, well, we intentionally wanted to take them from native site every time. How I think there's no need to do it. You could think about removing extracting from from Java and Objective-C if it's not big deal for you.
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. Hm, the constants are used in the native part as well. Hence it actually makes sense to read them in the JavaScript part from the native side. Should I undo the changes in
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. No, it's fine now |
||
| FAILED: 'FAILED', | ||
| BEGAN: 'BEGAN', | ||
| CANCELLED: 'CANCELLED', | ||
| ACTIVE: 'ACTIVE', | ||
| END: 'END', | ||
| }; | ||
|
|
||
| const Directions = {}; | ||
|
|
||
| // Factory for Handler components | ||
| function createHandler(name, attachNativeHandler) { | ||
| return class Handler extends React.Component { | ||
| static displayName = name; | ||
|
|
||
| container = React.createRef(); | ||
|
jaulz marked this conversation as resolved.
Outdated
|
||
| detachNativeHandler = null; | ||
|
|
||
| componentDidMount() { | ||
| // Get current DOM node | ||
| const node = findNodeHandle(this.container.current); | ||
|
|
||
| // Attach handler to DOM node | ||
| const { enabled } = this.props; | ||
| if (node && attachNativeHandler && enabled !== false) { | ||
| this.detachNativeHandler = attachNativeHandler(node, this.props); | ||
| } | ||
| } | ||
|
|
||
| componentDidUpdate() { | ||
| // Get current DOM node | ||
| const node = findNodeHandle(this.container.current); | ||
|
|
||
| if (node && attachNativeHandler) { | ||
| // Detach existing handlers | ||
| if (this.detachNativeHandler) { | ||
| this.detachNativeHandler(node); | ||
| } | ||
|
|
||
| // Attach handler to DOM node again | ||
| const { enabled } = this.props; | ||
| if (enabled !== false) { | ||
| attachNativeHandler(node, this.props); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| render() { | ||
| const { children, ...rest } = this.props; | ||
|
|
||
| // We don't want to create another layer, so instead we clone it only but keep the reference | ||
| const child = React.Children.only(children); | ||
| return React.cloneElement(child, { | ||
|
jaulz marked this conversation as resolved.
Outdated
|
||
| ref: this.container, | ||
| ...rest, | ||
| }); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| // Create all Handler components with their respective handler functions | ||
| // (at the moment only TapGestureHandler is properly supported) | ||
| const NativeViewGestureHandler = createHandler('NativeViewGestureHandler'); | ||
| const TapGestureHandler = createHandler('TapGestureHandler', (node, props) => { | ||
| const { onHandlerStateChange = () => {} } = props; | ||
| const clickHandler = ({ x }) => | ||
| onHandlerStateChange({ | ||
| nativeEvent: { | ||
| oldState: State.ACTIVE, | ||
| state: State.UNDETERMINED, | ||
| x, | ||
| }, | ||
| }); | ||
| node.addEventListener('click', clickHandler); | ||
|
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. Using these DOM events directly means you're not hooked into the responder event system
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. Do you have any link for me to see how we could improve this? 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. I'm not familiar with the behaviour/implementation of the native |
||
|
|
||
| // Detach event listeners | ||
| return () => { | ||
| node.removeEventListener('click', clickHandler); | ||
| }; | ||
| }); | ||
| const FlingGestureHandler = createHandler('FlingGestureHandler'); | ||
| const ForceTouchGestureHandler = createHandler('ForceTouchGestureHandler'); | ||
| const LongPressGestureHandler = createHandler('LongPressGestureHandler'); | ||
| const PanGestureHandler = createHandler('PanGestureHandler'); | ||
| const PinchGestureHandler = createHandler('PinchGestureHandler'); | ||
| const RotationGestureHandler = createHandler('RotationGestureHandler'); | ||
|
|
||
| // Factory for Button component | ||
| // (at the moment this is a plain TouchableWithoutFeedback) | ||
| function createButton(name) { | ||
| return class Button extends React.Component { | ||
| static displayName = name; | ||
|
|
||
| render() { | ||
| return <TouchableWithoutFeedback {...this.props} />; | ||
|
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. If these are meant to be used as buttons in the UI, you should add
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. Thanks for the hint! |
||
| } | ||
| }; | ||
| } | ||
|
|
||
| const RawButton = createButton('RawButton'); | ||
| const BaseButton = createButton('BaseButton'); | ||
| const RectButton = createButton('RectButton'); | ||
| const BorderlessButton = createButton('BorderlessButton'); | ||
|
|
||
| // Export same components as in GestureHandler.js | ||
| export { | ||
| ScrollView, | ||
| Slider, | ||
| Switch, | ||
| TextInput, | ||
| ToolbarAndroid, | ||
| ViewPagerAndroid, | ||
| DrawerLayoutAndroid, | ||
| WebView, | ||
| NativeViewGestureHandler, | ||
| TapGestureHandler, | ||
| FlingGestureHandler, | ||
| ForceTouchGestureHandler, | ||
| LongPressGestureHandler, | ||
| PanGestureHandler, | ||
| PinchGestureHandler, | ||
| RotationGestureHandler, | ||
| State, | ||
| /* Buttons */ | ||
| RawButton, | ||
| BaseButton, | ||
| RectButton, | ||
| BorderlessButton, | ||
| /* Other */ | ||
| FlatList, | ||
| gestureHandlerRootHOC, | ||
| Directions, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| // At the moment no swipe is supported | ||
| export default function Swipeable({ children }) { | ||
| return children; | ||
|
kmagiera marked this conversation as resolved.
|
||
| } | ||
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.
If
AnimatedEventneeds to be used directly, you may want to ask that it is added to theAnimatedexport from React Native rather than relying on the fragility of reaching into internals.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.
Okay, I will check on their repository and raise a PR.
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.
Will keep track here: react/react-native#22984