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

Unwanted behavior in a UIScrollView #177

Open
fhucho opened this issue Dec 5, 2020 · 4 comments
Open

Unwanted behavior in a UIScrollView #177

fhucho opened this issue Dec 5, 2020 · 4 comments

Comments

@fhucho
Copy link

fhucho commented Dec 5, 2020

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

  • With disablePanGestures == false, when I touch and move horizontally, the stars are being updated as expected. But as soon as I move slightly vertically, UIScrollView's pan gesture detector captures the touch gesture and CosmosView acts as if the touch ended.
  • With disablePanGestures == true, there's a different problem when I just want to scroll and start by touching the CosmosView - it will immediately capture the touch gesture.

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
Copy link
Author

fhucho commented Dec 5, 2020

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
Copy link
Owner

evgenyneu commented Dec 6, 2020

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

  • disablePanGestures setting was introduced recently as a simple workaround for having a Cosmos view on a modal screen Unable to swipe/drag between values with new iOS modal presentation style #148. Frankly, I don't want to use it for other purposes because it will make the code harder to understand and confuse people (including myself).

  • The main problem comes from having Cosmos view in a scroll view and allowing users to change the rating. This is by far the most frequently reported issue. Did you try disabling "Cancellable content touches" for the scrollview described here? It worked for some people but not everybody. I think there is a fundamental ambiguity in sharing touch input between both scroll view and Cosmos view. I would personally only use Cosmos for showing rating in a scroll view and not allow to change it there, just to preserve my sanity. 😂

  • I like your second workaround, this is a how a UI should behave: you only make changes when finger is lifted AND it is within the area of the Cosmos view (plus/minus some margins). I tried this idea in the past but my implementation had other unwanted side effects, so I dropped it. One thing to note here, updateOnTouch and disablePanGestures are supposed to be properties set by the user, and not by the library itself. If the code works for you then it's fine, but I don't want to implement this in the library because these two properties were not made for this and there can be side effects.

Anyway, very good stuff, thanks for sharing. 👍

@fhucho
Copy link
Author

fhucho commented Dec 6, 2020

Hey Evgenii,

  • Agreed, I just wanted to demonstrate the solution using as few lines of code as possible. Whether and how to implement in the Cosmos library is another thing.

  • Wasn't aware of "Cancellable content touches"... After I disabled it, it basically solves my problem, but there's one minor edge case. If I touch, wait for a brief moment and then move vertically, CosmosView receives touchesCancelled and acts as if I just selected a rating. What I want is a UIButton-like behavior - once CosmosView starts tracking a touch, the scroll view will never steal it.

  • Yeah, this is probably not how it should be implemented in the library.

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants