52inc / Pulley

A library to imitate the iOS 10 Maps UI.
https://cocoapods.org/pods/Pulley
MIT License
2.01k stars 264 forks source link

drawerPositionDidChange delegate method called multiple times in a row #161

Closed kcharwood closed 6 years ago

kcharwood commented 6 years ago

I'm looking to use a UIImpactGenerator in my app when the drawer hits different states, and this method seems like a good place to listen for that event. However, I'm seeing the delegate get called multiple times in a row for each event, rather than once (as I would expect).

Is this by design, or this a bug? I can work around this on my side by tracking last known state, and just ignoring states that match the last reported state, but it's not as clean as if the library just sent the callback once for each event.

Thanks!

amyleecodes commented 6 years ago

You should expect this method to be called frequently with duplicate values.

It's called in many scenarios where the drawer layout, position, or settings may have changed, or the size for PulleyViewController itself has been changed.

In addition to actual drawer position changes, you'll also see it called during these events (and potentially more):

None of these guarantee that the drawer position is different than the last time the delegate method was called. Instead, it indicates that your delegate method should make any UI adjustments necessary for the current state of the drawer as it may have changed. (Heights, sizes, layouts, positions, etc.). Think of it as 'sync UI to the current drawer state' callback rather than one for just literal position changes.

amyleecodes commented 6 years ago

Follow-up: I'd never considered adding Taptic Feedback support to Pulley. If this is something of interest, I'd be happy to add it to Pulley in an update later this evening and you can just enable it / configure it.

Let me know that would appeal to your use case.

Thanks!

kcharwood commented 6 years ago

Thanks for the detailed write up. That behavior definitely explains what I’m seeing.

In terms of behavior, I’d like to see a little thud every time the drawer animation completes for the various positions (and only when it animates to that spot, not in a non animation use case).

UIImpactFeedbackGenerator is probably the haptic class I would use... just need to decide what style to support based on feel. Maybe could be an optional property and I could set it externally based on what I want? When it’s set, you call ‘prepare’ then call ‘impactOccured’ internally every time the animation is complete? If the property isn’t set, it’s a no op.

public var feedbackGenerator: UIImpactFeedbackGenerator? {
  didSet {
      feedbackGenerator?.prepare()
      }
   }

Typed all this on my phone so forgive my typos.

amyleecodes commented 6 years ago

@kcharwood Support for UIFeedbackGenerator subclasses is now available in Pulley 2.2.5 on CocoaPods. You can see the setup for it in the sample project here: https://github.com/52inc/Pulley/blob/master/Pulley/DrawerContentViewController.swift#L46

The actual property on PulleyViewController is here: https://github.com/52inc/Pulley/blob/master/PulleyLib/PulleyViewController.swift#L426

Prepare only lasts a few seconds (in which time the user is unlikely to trigger it). So, instead, it prepares it when the user starts scrolling the drawer (in addition to immediately before each use).

Let me know if you have any questions!

skeeJay commented 5 years ago

This seems to add the haptic feedback in the animation block instead of the completion block (when the user lifts their finger instead of when the drawer "snaps" into its new position). I know you can add a callback to the completion block if the position is changing with setDrawerPosition, but is there a way to achieve the latter effect on manual drawer position change?