Skip to content
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

BUG: React Native Web - onGridClick() not sending correct parameters #244

Open
rdewolff opened this issue Jul 23, 2022 · 9 comments
Open

Comments

@rdewolff
Copy link

rdewolff commented Jul 23, 2022

Describe the bug
When using RNWV on react-native-web, when clicking on the grid, the parameters are not sent properly to the callback function.

Steps To Reproduce

  1. launch on the web
  2. register a onGridClick event
  3. param startHour is always set to 0
    ...

Expected behavior

  • param startHour should be set to the hour clicked.

Environment:

  • react-native-week-view: version 0.16.0
  • react-native: 0.63.2
  • react: 16.13.1
  • OS: iOS / not tested on Android
@pdpino pdpino added type: bug status: needs review Will be reviewed by a contributor labels Jul 25, 2022
@pdpino
Copy link
Collaborator

pdpino commented Jul 26, 2022

I haven't tried to reproduce this yet, will do soon.

@rdewolff have you tried with rn-week-view latest version? (v0.18.0)

@rdewolff
Copy link
Author

rdewolff commented Aug 1, 2022

Nah I have tested on 0.16.0 only yet.

@rdewolff
Copy link
Author

rdewolff commented Aug 1, 2022

Seems like 0.18 is not working at all on react-native-web.

Screenshot 2022-08-01 at 22 00 37

@pdpino
Copy link
Collaborator

pdpino commented Aug 2, 2022

I'm able to reproduce the error Cannot read properties of undefined (reading 'Tap') in web by using react-native-gesture-handler: "~1.10.2". This error disappears if you upgrade react-native-gesture-handler to v2 (e.g. 2.5.0) (v2 is required for week-view)

However, when upgrading RNGH to v2.5.0 I run into this other RNGH bug in web: software-mansion/react-native-gesture-handler#2056 (completely unrelated to week-view), so for now it won't work either :(

@pdpino
Copy link
Collaborator

pdpino commented Aug 2, 2022

I also reproduced the original issue (i.e. startHour being set to 0) in rn-week-view 0.16.0, it is a simple platform compatibility issue:

  • In android / ios the position is extracted from the native event as: nativeEvent: { locationY: <position> }, to get the hour correct
  • In web it should be: nativeEvent: { offsetY: <position> }

@pdpino
Copy link
Collaborator

pdpino commented Aug 2, 2022

@rdewolff I think week-view still has some issues to be fully supported in web (other example: scrolling #198).

For me, the priority right now is releasing some features for mobile. But supporting rn-web also would be awesome.

For anyone else interested, you can keep listing other necessary stuff to support rn-web (like this issue), and we can try to pick it up in the near future

@pdpino pdpino removed the status: needs review Will be reviewed by a contributor label Aug 2, 2022
@rdewolff
Copy link
Author

rdewolff commented Aug 8, 2022

It seems like on the web it's possible to use the event.nativeEvent.y prop to calculate the hour correctly.

@rdewolff
Copy link
Author

rdewolff commented Aug 8, 2022

On version 0.16.0, here is the working patched version if that can help:

diff --git a/node_modules/react-native-week-view/src/Events/Events.js b/node_modules/react-native-week-view/src/Events/Events.js
index 4b1cb13..17ce5e2 100644
--- a/node_modules/react-native-week-view/src/Events/Events.js
+++ b/node_modules/react-native-week-view/src/Events/Events.js
@@ -1,6 +1,6 @@
 import React, { PureComponent } from 'react';
 import PropTypes from 'prop-types';
-import { View, TouchableWithoutFeedback } from 'react-native';
+import { View, TouchableWithoutFeedback, Platform } from 'react-native';
 import moment from 'moment';
 import memoizeOne from 'memoize-one';
 
@@ -197,8 +197,15 @@ class Events extends PureComponent {
     }
     const { initialDate, hoursInDisplay, beginAgendaAt } = this.props;
 
+    // support different `nativeEvent` for browser 
+    let yValue
+    if (Platform.OS === 'web') {
+      yValue = event.nativeEvent.offsetY
+    } else {
+      yValue = event.nativeEvent.locationY
+    }
     const seconds = yToSeconds(
-      event.nativeEvent.locationY - CONTENT_OFFSET,
+      yValue - CONTENT_OFFSET,
       hoursInDisplay,
       beginAgendaAt,
     );

@pdpino
Copy link
Collaborator

pdpino commented Aug 8, 2022

@rdewolff are you staying in 0.16.0 in web and mobile? only web?

I'm wondering if is worth to make reanimated and gesture-handler optional dependencies, though it might be too much trouble

EDIT with more info: rn-reanimated and rn-gesture-handler handle these kind of web/mobile differences internally, so >= v0.17.0 solves this specific problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants