search

mkko / DrawerView

A drop-in view, to be used as a drawer anywhere in your app
MIT License
375 stars 57 forks source link

drawer(_ drawerView: DrawerView, didTransitionTo position: DrawerPosition) delegate method not always called #31

Closed logananderson closed 3 years ago

logananderson commented 4 years ago

Just diving into this bug I have found. In my app, I want to change logic depending if the user has swiped the drawer closed. I basically pop up a UI element that allows them to reopen the drawer since closed in my app is completely closed. I have this logic in the drawer(_ drawerView: DrawerView, didTransitionTo position: DrawerPosition) delegate method checking if position == .closed. I have found that if the user swiped multiple times in quick succession, this delegate method will not fire even though the drawer does indeed transition to the .closed position. willTransitionTo does fire, so as a workaround I am changing to listen to that.

Steps to reproduce:

  1. Clone and run DrawerViewExample app
  2. Run the app on a physical device (easier to reproduce this way, but can also happen in sim)
  3. Swipe the drawer up multiple times in quick succession
  4. Observe console logs

Expected: (These logs are from swiping up once)

drawerWillBeginDragging
drawerWillEndDragging
drawer(_:willTransitionFrom: partiallyOpen to: open)
drawerView(_:didTransitionTo: open)

Actual:

drawerWillBeginDragging
drawerWillEndDragging
drawer(_:willTransitionFrom: partiallyOpen to: open)
drawerWillBeginDragging
drawerWillBeginDragging
drawerWillBeginDragging
drawerWillBeginDragging

In collecting these logs, I notice that there are several drawerWillBeginDragging events without corresponding drawerWillEndDragging events. Perhaps this is a good starting point.

mkko commented 4 years ago

Hi and thank you for this! I briefly looked into this and I was able to reproduce the issue. I think the issue is caused by the fact that the position has already changed while dragging the drawer again. There's a line

let notifyDelegate = !_isConcealed && (currentPosition != visiblePosition)

which was supposed to make sure that there's no redundancy in delegate calls. I'm not absolutely certain but I think this might be the reason.

The whole delegate call is a bit of a mess and and afterthought, so I'll have to look into this a bit more.

mkko commented 3 years ago

Hey, is this issue still valid? I tried reproducing without success.

I'm closing this for now, please reopen if still valid.