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

Add tap delegate for tap/double tap detection #293

Merged
merged 1 commit into from
Oct 9, 2022
Merged

Add tap delegate for tap/double tap detection #293

merged 1 commit into from
Oct 9, 2022

Conversation

dknchris
Copy link
Contributor

@dknchris dknchris commented Oct 2, 2022

A reliable tap event helps users hide/unhide navigation bar or toolbars to give a full unobstructed view of the image.

A reliable tap event helps users hide/unhide navigation bar or toolbars to give a full unobstructed view of the image.
@dknchris
Copy link
Contributor Author

dknchris commented Oct 2, 2022

LightboxControllerTapDelegate essentially replaces LightboxControllerTouchDelegate for a few reasons:

  1. LightboxControllerTouchDelegate function is called when we 'tap' the page and not really every time we 'touch/drag'. Hence a function with didTap makes more sense with the newLightboxControllerTapDelegate.
  2. LightboxControllerTouchDelegate didTouch function isn't fired when the image is zoomed making it impossible to show/hide navigation bar or a toolbar when we tap a zoomed image. This is fixed by the new delegate which supports both tap/double tap events regardless of the zoom state. This is exactly what is done in iOS Photos app.

But removing LightboxControllerTouchDelegate is obviously a breaking change so we could either keep both the touch and the tap delegates and deprecate touch delegate until a major revision.

Also, thank you for your work!

@3lvis
Copy link
Collaborator

3lvis commented Oct 9, 2022

Thank you for your contribution, @dknchris!

@3lvis 3lvis merged commit 44c91b4 into hyperoslo:master Oct 9, 2022
@dknchris dknchris deleted the tap-delegate branch October 11, 2022 13:07
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

Successfully merging this pull request may close these issues.

None yet

2 participants