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

Refershing/Refresh Control #108

Closed
dandresfg opened this issue Jun 18, 2021 · 10 comments · Fixed by #112
Closed

Refershing/Refresh Control #108

dandresfg opened this issue Jun 18, 2021 · 10 comments · Fixed by #112
Labels
type: feature enhancements or new features

Comments

@dandresfg
Copy link

Any possibilities of add this ?

@pdpino
Copy link
Collaborator

pdpino commented Jun 18, 2021

Hi @dandresfg , could you be more specific on what do you need?

If you update the events prop, the events get rendered again. Is this what you mean by refreshing?

@dandresfg
Copy link
Author

dandresfg commented Jun 18, 2021

i mean <ActivityIndicator />. onRefresh in a virtualizedList to reload all events (as example)

@pdpino
Copy link
Collaborator

pdpino commented Jun 19, 2021

I see. I think we could add a prop isRefreshing to show an ActivityIndicator in the middle of the week-view grid. However, we cannot add a "pull to refresh" behavior with onRefresh, as the week-view grid has infinite scrolling. We could also add a refreshControl to customize the spinner component.

@hoangnm what do you think of a isRefreshing prop? I think is very straight-forward to implement.


@dandresfg as a workaround, for now you can try this (somewhat hacky) solution to have a spinner.

const { height: SCREEN_HEIGHT, width: SCREEN_WIDTH } = Dimensions.get('window');
const PADDING = 60; // Value defined inside the library
const CONTAINER_HEIGHT = SCREEN_HEIGHT - PADDING;
const CONTAINER_WIDTH = SCREEN_WIDTH - PADDING;

let isRefreshing = true; // could be state, prop, etc

// ...

<View style={{ flex: 1 }}>
  {isRefreshing && (
    <ActivityIndicator
      style={{
        position: 'absolute',
        top: CONTAINER_HEIGHT / 2,
        right: CONTAINER_WIDTH / 2,
        zIndex: 1,
      }}
      // ... more props
    />
  )}
  <WeekView
    events={events}
    // ... more props
  />
</View>

@pdpino
Copy link
Collaborator

pdpino commented Jul 12, 2021

@dandresfg regarding your comment on the PR:

this is only an activity indicator. I mean scroll up at the top. Just like to refresh a page in chrome.

As I said in the previous comment, I was not adding the "pull to refresh" behavior for the moment.

  • Horizontally: it cannot be done (infinite scrolling)
  • Vertically (i.e. scroll up at the top): I do not agree with this feature for this library. If the user is looking at 4pm, they have to scroll up all the way to 00:00 am to refresh. I think it makes more sense to handle the refreshing outside this library, by providing a different way to refresh (e.g. a button), like the Android Calendar.

For now, with the code from the PR, you can set the isRefreshing prop to true to display a spinner, and handle the isRefreshing=true/false outside the WeekView component (e.g. with a refresh-button)

Maybe later we can try adding a callback for the "scroll up at the top" action, something like onScrollAtTop={() => {...}}, but I'm not sure if this is possible (by looking at ScrollView props).

@dandresfg
Copy link
Author

Sorry, get lost and i wrote in the PR.

I mean, i edited the library to start at 7:00am (Other thing that could be awesome is starting hour, hiding all the previous hours) I got a refreshing, thanks for your code, works great (i put outside the library). I want to add the pull to refresh i tried manually but doesnt work (dunno why)

How can i added it by myself ? (and looking for some ideas, cuz the traditional one doesnt work as expected)

@pdpino
Copy link
Collaborator

pdpino commented Jul 13, 2021

I setup the pull-to-refresh feature in this experimental branch, should be more or less what you need. See the latest commit with the code modifications. You can start from this branch and make more custom changes as needed.

The result looks like this:

experimental-scrollview-on-refresh.mp4

This change is not compatible with the #112 PR, so it will remain only as an experimental branch for now.

@pdpino
Copy link
Collaborator

pdpino commented Jul 13, 2021

i edited the library to start at 7:00am (Other thing that could be awesome is starting hour, hiding all the previous hours)

Regarding this topic, the #70 issue asks for a similar feature (end-hour at the bottom). The full implementation I'd say is tricky, there are multiple edge cases to handle, and more.

If you already implemented it, you are welcome to send a PR or share the code, if you'd like!
If so, I kindly ask you to move the discussion to that issue or create a new one 😃

@dandresfg
Copy link
Author

The video is not playable (same as the issue onSwipeNext twice, it isnt playable too). Its just like the video is corrupted or deleted

I setup the pull-to-refresh feature in this experimental branch, should be more or less what you need. See the latest commit with the code modifications. You can start from this branch and make more custom changes as needed.

The result looks like this:

experimental-scrollview-on-refresh.mp4
This change is not compatible with the #112 PR, so it will remain only as an experimental branch for now.

I will take a look as soon as possible.

@pdpino
Copy link
Collaborator

pdpino commented Jul 13, 2021

The video is not playable (same as the issue onSwipeNext twice, it isnt playable too). Its just like the video is corrupted or deleted

Are you using firefox/opera? Apparently those browsers cannot reproduce it, but chromium based browsers can (chrome, brave, etc).

I attach a similar gif here:

experimental-scrollview-on-refresh

@dandresfg
Copy link
Author

dandresfg commented Jul 13, 2021

Opera, but yeah i think this is what im looking for

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature enhancements or new features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants