isaachinman / wundershine-native-app

0 stars 0 forks source link

Modify ImageQueue reorder animation duration based on distance travelled #114

Closed isaachinman closed 6 years ago

isaachinman commented 6 years ago

Is your feature request related to a problem? Please describe. From Mick:

When creating the mockup animation I made sure that the "moving" image section is visible at the start of the move with an ease in / slow start effect. See mockup video from 0:24 https://www.wundershine.com/nl/videos/Wundershine_Android_Proto.mp4

This will give the user visual confirmation that the picture is not removed, but moving to a different slot. Currently it moves too fast to see when the distance to travel is a longer / queue is longer. The animation speed / curve needs to be adjusted and perhaps layering / z depth. I can't really see the layering now.

Describe the solution you'd like We must calculate the distance travelled (distance in index of two items), and use this as a multiplier for the layout animation duration.

Describe alternatives you've considered N/A.

Additional context This calculation will need to happen in the select and deselect actions. Eg here.

isaachinman commented 6 years ago

So, I've engineered the following solution:

  1. All select/deselect animations start with a value of 300ms
  2. Distance travelled (number of ranks, absolute value) x 30ms will be added to the above
  3. A maximum total animation time of 800ms will not be exceeded, ever

This is not the most ideal solution, but I think it's what we'll have to live with. The limiting factor is that we're using React Native's built-in LayoutAnimation, which means the entire layout is animating for one specific duration.

If you log into an account with a massive queue, and deselect the top item: the time it takes for that top item to reach the very bottom of the list must be the same amount of time it takes for the top edge of the second item to reach the spot where the top edge of the first item previously was.

Hopefully that makes sense, and demonstrates our limitation. There will be animation degradation as queues exceed 40-50 items, in that the item which has been selected or deselected will move exceedingly fast. Not much to be done about that without handling animations in-house.

isaachinman commented 6 years ago

69fe7cb

Tintels commented 6 years ago

Explanation is clear.

Two things:

  1. Are the animatinos al linear? Difficult to see. It looks like the queue is easing as it is "filling the gap" but not sure about the start of the moving single image. Perhaps that could keep the moving image visible in long queues as it leaves its position. Even within the described duration limitations.

  2. The deselect looks smooth and the moving image is clearly visible in front of the layout as it leaves it's position. The select action is less smooth / visible. It seems that not all elements of the moving "image cell" are in front of the queue. Perhaps only the thumbnail. As it moves up, the thumbnails higher in the queue are flashing a bit during the transition.

isaachinman commented 6 years ago
  1. No, they are ease-in-out. We discussed this previously somewhere.
  2. Yes, that reflects an ongoing problem within React Native regarding zIndex.

Here's the way we'd approach the elevation/zIndex issue if it became possible:

@action
selectImage = async (imageID) => {
  this.setImageLoading(imageID)
  this.setImageRaised(imageID)
  try {
    const res = await apiRequest({ url: `/pv/queue/${this.queueType}/images/select`, data: { imageID } })
    const { data } = res

    const distanceTravelled = Math.abs(this.data.images.findIndex(i => i._id === imageID) -
      [...data.selectedImages, ...data.deselectedImages].findIndex(i => i._id === imageID))
    const animationDuration = Math.min((300 + (distanceTravelled * 30)), 800)

    this.mergeIntoLocalData(data, true, animationDuration)
    setTimeout(() => this.removeImageRaised(imageID), animationDuration)

  } catch (error) {
    runInAction(() => this.error = error)
    this.removeImageRaised(imageID)
  }
  this.removeImageLoading(imageID)
}

The closest workaround I could find is in this issue, where I've attempted to implement it and failed. We'll see if anyone follows up. Otherwise, this may have to wait. It's a relatively small issue nonetheless.

Tintels commented 6 years ago

OK. Shall we put this issue on backlog as reminder for when RN team has solution?

isaachinman commented 6 years ago

No. This issue is solved. The animation time now includes a distance-travelled modifier.

If you want to open another issue regarding animation zIndex, feel free.