evgenyneu / Cosmos

A star rating control for iOS/tvOS written in Swift
MIT License
2.18k stars 369 forks source link

Unwanted behavior in a UIScrollView #177

Open fhucho opened 3 years ago

fhucho commented 3 years ago

When I put a CosmosView into a UIScrollView, here's how it responds to touches:

Ideally, it should behave like UIButton does: when I touch and immediatelly start moving vertically, UIScrollView gets the gesture. Otherwise, the UIButton gets it.

Here's how it can be done by extending CosmosView. The idea is that UIScrollView's pan detector is disabled only after CosmosView starts receiving touches, which by default happens after short delay, during which the UIScrollView has a chance to recognize a pan gesture.

class MyCosmosView: CosmosView {
  open override func touchesBegan(_ touches: Set<UITouch>, with event: UIEvent?) {
    self.settings.disablePanGestures = true
    super.touchesBegan(touches, with: event)
  }

  open override func touchesEnded(_ touches: Set<UITouch>, with event: UIEvent?) {
    self.settings.disablePanGestures = false
    super.touchesEnded(touches, with: event)
  }

  open override func touchesCancelled(_ touches: Set<UITouch>, with event: UIEvent?) {
    self.settings.disablePanGestures = false
    super.touchesCancelled(touches, with: event)
  }
}
fhucho commented 3 years ago

Also, UIButton has another behavior that would be nice in a CosmosView - when the user starts touching and moves away, the button gets visually "unpressed" and lifting the finger doesn't trigger a tap. Moving back to the button presses it again.

Here's my implementation. When the distance between touch pointer and CosmosView's frame exceeds 80, I stop updating the CosmosView and reset the rating to 0.

class MyCosmosView: CosmosView {
  private let maxTouchDistance = 80.0

  open override func touchesBegan(_ touches: Set<UITouch>, with event: UIEvent?) {
    self.settings.disablePanGestures = true
    super.touchesBegan(touches, with: event)
  }

  open override func touchesMoved(_ touches: Set<UITouch>, with event: UIEvent?) {
    guard let location = touches.first?.location(in: self) else { return }
    let distance = max(-location.x, -location.y, location.x - self.bounds.width, location.y - self.bounds.height)

    if Double(distance) > self.maxTouchDistance {
      self.settings.updateOnTouch = false
      self.rating = 0
    } else {
      self.settings.updateOnTouch = true
    }

    super.touchesMoved(touches, with: event)
  }

  open override func touchesEnded(_ touches: Set<UITouch>, with event: UIEvent?) {
    self.settings.disablePanGestures = false
    self.settings.updateOnTouch = true
    if self.rating > 0 {
      super.touchesEnded(touches, with: event)
    }
  }

  open override func touchesCancelled(_ touches: Set<UITouch>, with event: UIEvent?) {
    self.settings.disablePanGestures = false
    self.settings.updateOnTouch = true
    if self.rating > 0 {
      super.touchesCancelled(touches, with: event)
    }
  }
}
evgenyneu commented 3 years ago

Hi @fhucho thanks for reporting the bugs and for the workarounds. Some thoughts/comments:

Anyway, very good stuff, thanks for sharing. 👍

fhucho commented 3 years ago

Hey Evgenii,

To be honest, I don't think an interactive rating bar in a scroll view is wrong from a UX perspective :) Interactive views are common in scroll views - buttons, swipeable cells in a table view or even a rating bar in the Google Maps app.

evgenyneu commented 3 years ago

To be honest, I don't think an interactive rating bar in a scroll view is wrong from a UX perspective :) Interactive views are common in scroll views - buttons, swipeable cells in a table view or even a rating bar in the Google Maps app.

Good point, agreed.