hoangnm / react-native-week-view

Week View Component for react-native
MIT License
322 stars 96 forks source link

The drag is not working as expected when user is accidentally placing it in between the lines #239

Closed TheTushar2696 closed 2 years ago

TheTushar2696 commented 2 years ago

When the user is placing the event in between the lines and releases it the event becomes unresponsive from the right side of the line, Sharing a video of the same.

https://user-images.githubusercontent.com/40525668/179201612-7b04dcd9-c30c-465a-a1e3-f5b99788fc8e.mov

pdpino commented 2 years ago

Can you provide your code? Especially the onDragEvent callback.

It seems the DB is not being properly updated in onDragEvent. The expected behavior: user drops the event --> the box snaps and aligns with the day

TheTushar2696 commented 2 years ago
const showAlertOnUpdate = (event, newStartDate, newEndDate, objectToUpdate) => {
    Alert.alert(t('task.saveChangesFor'), undefined, [
      {
        text: t('task.onlyThisEvent'),
        onPress: () => {
          roundOffTimeWhenDraggedLessThan15Minutes(event, newStartDate, newEndDate, false)
        },
      },
      {
        text: t('task.allEvents'),
        onPress: () => {
          roundOffTimeWhenDraggedLessThan15Minutes(event, newStartDate, newEndDate, true)
        },
      },
      {
        text: t('cancel'),
        onPress: () =>
          updateLocalState(event, event.startDate, event.endDate, objectToUpdate, false, false),
      },
    ])
    return
  }

  const updateLocalState = (
    event,
    newStartDate,
    newEndDate,
    objectToUpdate,
    allEvents,
    callApi,
  ) => {
    let updatedWeekViewTask = []
    setCalendarLoading(true)
    if (allEvents) {
      // let recurringTasksInLocalState = weekViewTasks.filter(recurringTask => {
      //   return event.recurringTaskId === recurringTask.recurringTaskId
      // })
      let timeDiff = event.startDate - newStartDate
      console.log(timeDiff, 'trtr')
      for (let i = 0; i < weekViewTasks.length; i++) {
        if (event.recurringTaskId === weekViewTasks[i].recurringTaskId) {
          weekViewTasks[i].startDate = new Date(weekViewTasks[i].startDate.getTime() + timeDiff)
          weekViewTasks[i].endDate = new Date(
            weekViewTasks[i].endDate.getTime() - weekViewTasks[i].startDate.getTime() + timeDiff,
          )
        }
        updatedWeekViewTask.push(weekViewTasks[i])
      }
    } else {
      for (let i = 0; i < weekViewTasks.length; i++) {
        if (event.id === weekViewTasks[i].id) {
          weekViewTasks[i].startDate = newStartDate
          weekViewTasks[i].endDate = newEndDate
        }
        updatedWeekViewTask.push(weekViewTasks[i])
      }
    }
    setWeekViewTasks(updatedWeekViewTask)
    if (callApi) {
      setCalendarLoading(true)
      updateEvent(event, newStartDate, newEndDate, allEvents)
    }
    setCalendarLoading(false)
  }

  const updateEvent = async (event, newStartDate, newEndDate, allEvents) => {
    let tempTask = {
      startAtUtc: newStartDate,
      endAtUtc: newEndDate,
      notifyAtUtc: newStartDate,
    }
    console.log('klkl', newStartDate, newEndDate)
    // setLoading(true)
    await updateTaskById(event.id, event.recurringTaskId, tempTask, allEvents, true)
      .then(() => {
        setCalendarLoading(false)
      })
      .catch(e => {
        setCalendarLoading(false)
        setCalendarLoading(false)
        handleApiFailure(e)
      })
  }

  const roundOffTimeWhenDraggedLessThan15Minutes = (event, newStartDate, newEndDate, allEvents) => {
    // console.log('klkl')
    let finalNewStartDate = newStartDate
    let finalNewEndDate = newEndDate
    // const diffMs = event.startDate - newStartDate
    // const diffDays = Math.floor(diffMs / 86400000)
    // const diffMins = ((diffMs % 86400000) % 3600000) / 60000
    // if (Math.abs(diffDays) === 0) {
    //   if (Math.abs(diffMins) < 15) {
    //     let minutesToAdd = Math.round(diffMins)
    //     finalNewStartDate = new Date(event.startDate.getTime() + minutesToAdd * 60000)
    //     finalNewEndDate = new Date(event.startDate.getTime() + minutesToAdd * 60000)
    //   }
    // }
    updateLocalState(event, finalNewStartDate, finalNewEndDate, {}, allEvents, true)
    // updateEvent(event, finalNewStartDate, finalNewEndDate, allEvents)
  }

  const onDragEvent = (event, newStartDate, newEndDate) => {
    if (newEndDate === newStartDate) {
      return
    }
    console.log(event, newStartDate, newEndDate, 'koko')
    var objectToUpdate = originalTasks.filter(obj => {
      return obj.id === event.id
    })
    if (objectToUpdate.length) {
      if (objectToUpdate[0].recurringTaskId) {
        showAlertOnUpdate(event, newStartDate, newEndDate, objectToUpdate)
      } else {
        updateLocalState(event, newStartDate, newEndDate, objectToUpdate, false, true)
      }
    }
  }
TheTushar2696 commented 2 years ago

These are the functions, starting from onDrag event

TheTushar2696 commented 2 years ago

@pdpino Actually what I want to achieve is,let the user drag the event wherever he wants and update the local states and silently call the APis and update the DB whereas if the api fails, restore the position of the event with an error message, and all these calculations leading to this, please suggest a better way to do it, Thanks in advance

pdpino commented 2 years ago

@TheTushar2696 what do you provide as events prop to <WeekView>?

Is not clear to me by checking the code when that prop gets updated

pdpino commented 2 years ago

(Sorry for the long answer)

There are two possible options/scenarios to update the events prop:

1. Sync DB update

If you are updating a DB locally in the mobile device (e.g. redux, or other store), you can choose to do this syncly. For this scenario, check this code:

const useEvents = (initialEvents = []) => {
  const [events, dispatch] = useReducer(
    (prevEvents, action) => {
      switch (action.type) {
        case 'updateEvent':
          const {event, newStartDate, newEndDate} = action.payload;
          return [
            // not the most efficient, probably you will use a library with a more efficient update method
            ...prevEvents.filter(e => e.id !== event.id),
            {
              ...event,
              startDate: newStartDate,
              endDate: newEndDate,
            },
          ];
        default:
          break;
      }
      return prevEvents;
    },
    initialEvents,
  );

  const updateEvent = ({ event, newStartDate, newEndDate }) => dispatch({
    type: 'updateEvent',
    payload: { event, newStartDate, newEndDate },
  });

  return {
    events,
    updateEvent,
  };
};

const MyComponent = () => {
  const {events, updateEvent} = useEvents();

  return (
    <WeekView
      events={events}
      onDragEvent={(event, newStartDate, newEndDate) => {
        // Here you must update the event in your DB with the new date and time
        updateEvent({ event, newStartDate, newEndDate }) // Sync call that triggers a re-render --> ok
      }}
    />
  );
}

https://user-images.githubusercontent.com/20805451/179325264-86b6fbce-54cb-4f0a-ba12-32341f743ee8.mp4

The interaction looks like this:

2. Async DB update

If you need to call a remote API with a http request, you'll need an async update. In other use cases an async call may also work best for you. (We have mainly tested the component using the sync scenario, so I'm not sure what will work best here)

Async option 1 (not working properly, UI looks jumpy)

Ideally, the solution would look something like this:

const useRemoteEvents = (initialEvents = []) => {
  const [{events, isLoading}, dispatch] = useReducer(
    (prevState, action) => {
      const {events: prevEvents} = prevState;
      switch (action.type) {
        case 'updateEvent':
          const {event, newStartDate, newEndDate} = action.payload;
          return {
            events: [
              // again, you probably want to use something lighter
              ...prevEvents.filter(e => e.id !== event.id),
              {
                ...event,
                startDate: newStartDate,
                endDate: newEndDate,
              },
            ],
            isLoading: false,
          };
        case 'startLoading':
          return {events: prevEvents, isLoading: true};
        case 'stopLoading':
          return {events: prevEvents, isLoading: false};
        default:
          break;
      }
      return prevState;
    },
    evts => ({events: evts, isLoading: false}),
    initialEvents,
  );

  const updateEvent = ({event, newStartDate, newEndDate}) =>
    dispatch({
      type: 'updateEvent',
      payload: {event, newStartDate, newEndDate},
    });

  const startLoading = () => dispatch({type: 'startLoading'});
  const stopLoading = () => dispatch({type: 'stopLoading'});

  return {
    events,
    isLoading,
    startLoading,
    stopLoading,
    updateEvent,
  };
};

const MyComponent = () => {
  const {events, isLoading, startLoading, stopLoading, updateEvent: updateLocalDB} = useRemoteEvents();

  const onDragEvent = (event, newStartDate, newEndDate) => InteractionManager.runAfterInteractions(async () => {
    startLoading(); // let the user know is loading // UI looks jumpy: see problem below
    // Here you want to make the async call before updating the DB locally
    const result = await updateRemoteDBWithHTTPRequest();

    if (result.success) {
      updateLocalDB({event, newStartDate, newEndDate});
    } else {
      stopLoading();
    }
  });

  return (
    <>
      <WeekView
        events={events}
        onDragEvent={onDragEvent}
        isRefreshing={isLoading} // default feedback
      />
      // or provide your own user feedback:
      <LoadingSpinner isLoading={isLoading} />
    </>
  );
}

https://user-images.githubusercontent.com/20805451/179325280-2b6a8fc1-e637-4559-a3a5-156ebc64cf6d.mp4

Problem:

Async option 2 (workaround)

As a workaround, you can apply the update locally, and then roll it back if there is an error with the API

const MyComponent = () => {
  const {events, updateEvent: updateLocalDB} = useRemoteEvents();

  const onDragEvent = (event, newStartDate, newEndDate) => {
    InteractionManager.runAfterInteractions(async () => {
      const result = await updateRemoteDBWithHTTPRequest(...);
      if (!result.success) {
        // rollback change
        updateLocalDB({ event, newStartDate: event.startDate, newEndDate: event.endDate });
        Alert.alert(`Event ${event.id} cannot be moved`);
      }

      // or do something more robust: upsert in the local DB the event from the API
    });

    // for now make the change locally, assume it will be accepted
    updateLocalDB({event, newStartDate, newEndDate});
  };

  return (
    <WeekView
      events={events}
      onDragEvent={onDragEvent}
    />
  );
}

Looks like this:

https://user-images.githubusercontent.com/20805451/179325294-95e6ec84-9ce3-41a4-9407-1f5b80bb6201.mp4

pdpino commented 2 years ago

Actually what I want to achieve is,let the user drag the event wherever he wants and update the local states and silently call the APis and update the DB whereas if the api fails, restore the position of the event with an error message

You probably want the second scenario

Lmk if you have any feedback or suggestions, we want to fully support the async scenario as well

TheTushar2696 commented 2 years ago

Hi @pdpino ,

Screenshot 2022-07-16 at 4 15 53 PM

pdpino commented 2 years ago

@TheTushar2696 performance is out of scope for this issue, but I can comment this:

pdpino commented 2 years ago

I'm moving the async discussion to #248 and marking the original question as solved (feel free to re-open if needed)