search

hoangnm / react-native-week-view

Week View Component for react-native
MIT License
309 stars 97 forks source link

React Native on macOS or on web - broken width and month not changing #198

Open rdewolff opened 2 years ago

rdewolff commented 2 years ago

The library is not working when compiling a react-native project for macOS (Catalyst for example). It's not working either when using react-native-web version.

The following elements are not working:

  1. The width of the day header is not correct
  2. The scroll is hard to test in these conditions, but it seems to not be able to trigger a next month change when you scroll past a certain amount of time horizontally.

Please find a video illustrating the first examples:

react-native-web-week-view-480

Steps to reproduce:

Notes

pdpino commented 2 years ago

@rdewolff I'll check this out ASAP

In the broken web app, where are events placed in the screen? For example an event in the central week, the week before and the week after. Could you show a screenshot/gif with those?

rdewolff commented 2 years ago

I haven't added meets on the first test I did. Let me give it a try and get back to u

pdpino commented 2 years ago

@rdewolff I was able to reproduce it and display some dummy events, so don't worry:

image

rdewolff commented 2 years ago

Aaah great, you got a setup working 🥳

rdewolff commented 2 years ago

I just tried to display some events in the grid but did not get it working. Did you already do some fixes @pdpino to get your event displayed?

pdpino commented 2 years ago

TL;DR: I fixed some things, but is still not working 100%, probably due to an upstream bug from react-native-web VirtualizedList. Demo:

fixed-header

(btw the cursor is not moving because I was scrolling with the wheel)

Details:

There are multiple things in the initial example that were wrong:

  1. Header does not have the appropriate width
  2. All the grey horizontal grid lines are rendered at 00:00hrs
  3. Vertical scrolling works, but the initial scroll to the startHour is not performed (i.e. you cannot call scrollTo() for the ScrollView ref)
  4. Horizontal scrolling is broken 4.1 The initial position should be today (in my example would be May 6), but the initial position shows April 21 4.2 You cannot scroll past certain dates to the right or left
  5. When scrolling the grid horizontally, the header does not follow the grid smoothly (i.e. animated)

What I did:

  1. :+1: Compute the header width using the size of the window
  2. :+1: Grey gridlines are now properly rendered throughout the grid (was a simple flex issue)
  3. :+1: The initial vertical position is fixed, and the vertical scroll works properly
    • Note: this required constraining the ScrollView's height to height: '100vh' (100 viewport's height, web only)
  4. VirtualizedList and FlatList have shown issues in react-native-web (for example see a list of issues here: https://github.com/necolas/react-native-web/pull/2241#issuecomment-1055824946)
    • 4.1. :+1: The initialScrollIndex behavior was fixed --> the week starts pointing to today (see gif)
    • 4.2 :-1: :-1: The scrolling is still constrained to 5 pages (weeks), from April 18 to May 22 (see gif)
      • Culprit: the onMomentumScrollEnd event from the horizontal VirtualizedList is not being triggered
      • We need that event to add pages (weeks) dynamically to the left and right
      • In the first render, we show 5 pages (those are the only pages shown in the gif)
      • I tested a small VirtualizedList example, and I think this is an issue from react-native-web (I can provide more details for a small repro)
  5. :-1: Could not make the animation smoother (though, the horizontal scrolling should be resolved first anyway)

4.2 is probably a dealbreaker for most apps. We should corroborate if it is a bug from react-native-web; hopefully they will have a workaround soon

pdpino commented 2 years ago

Did you already do some fixes to get your event displayed?

@rdewolff tbh I don't remember if I had already made a small fix for that demo :laughing:

I made a draft PR with the fixes so far

rdewolff commented 2 years ago

Concerning 4.2, is there a way to use an alternative to onMomentumScrollEnd?

pdpino commented 2 years ago

The options I can think of are not good, e.g.

rdewolff commented 2 years ago

So our best bet might be to wait for react-native-web to support onMomentumScrollEnd.

Maybe this comment is related to our issue: https://github.com/expo/expo/issues/16822#issuecomment-1099169366 ?

rdewolff commented 2 years ago

How can we get this issue moving forward?

pdpino commented 2 years ago

How can we get this issue moving forward?

PR #201 will fix up to 4.1.

As to 4.2, I'm afraid without scroll events in web there is not much more we can do. I see a lot of attention in rn-web, (e.g. https://github.com/necolas/react-native-web/issues/2249, https://github.com/necolas/react-native-web/pull/2241), hopefully they will have a fix soon

On the side, I'm implementing some performance improvements, and that might include migrating to recycler-list-view, which should support rn-web scroll events. Although these changes are taking me a bit longer. (I'll probably open a separate issue regarding this soon). UPDATE: See #209, we are not migrating to another library for now.

rdewolff commented 2 years ago

PR #201 was merged 🥳 https://github.com/necolas/react-native-web/issues/2249 is still open 👀 https://github.com/necolas/react-native-web/pull/2241 is done ✅

pdpino commented 2 years ago

@rdewolff necolas (rn-web 's author) commented:

those of you who need these events will need to implement and submit PRs for. I do not plan to work on adding these features myself.

I cannot work on that right now.

To recap, I think the options are:

  1. wait for react-native-web to support scroll events

or wait for other libraries, e.g.:

  1. react-native-bidirectional-infinite-scroll, does not support horizontal
  2. recyclerlistview, needs bidirectional scrolling

with 2 or 3 we could try to make the change only for web (instead of web and mobile), if that's simpler. In any case, have in mind is a big refactor and may cause conflicts with other stuff (animations, etc)

rdewolff commented 2 years ago

The current workaround is to add navigation button to navigate to previous or next week.

rdewolff commented 1 year ago

Just found this https://github.com/Flipkart/recyclerlistview Is it something we could use @pdpino ?

pdpino commented 1 year ago

Sorry for the late reply.

@rdewolff recyclerlistview does not support bidirectional infinite scrolling: https://github.com/Flipkart/recyclerlistview/pull/647#issuecomment-1258695663

They implemented a fix for VirtualizedList, and is published in a fork of react-native: https://github.com/facebook/react-native/pull/29466#issuecomment-1255383709. Using this might fix the problem?