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

QuickLook swipe down and less message motion when coming back from GiveBackMyFirstResponder #2482

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Amzd
Copy link
Collaborator

@Amzd Amzd commented Jan 5, 2025

This PR makes the image preview (quicklook) able to be dismissed by swiping down, instead of the default popGestureRecognizer. This is an improvement because A; the popGesture is only enabled when the navigation bars are shown, B; it interferes with swiping to previous image, and C; the animation feels good.

For this to look good I had to reduce the motion on messages when presenting/dismissing view controllers some more. Now after a view is dismissed the scroll updating is delayed by 0.5s which means that first; the ChatViewController can become firstResponder (bringing up the accessory) and then; the text field can become firstResponder (bringing up the keyboard) without the chat scrolling.

Quick Look Dismiss (This change is only pre-iOS 18)

Before After
RPReplay_Final1736090240.mp4
RPReplay_Final1736090333.mp4

Reduced motion (This change is everywhere)

Before After
RPReplay_Final1736090019.mp4
RPReplay_Final1736089664.mp4

@Amzd Amzd added the enhancement actually in development, user visible enhancement label Jan 5, 2025
@Amzd Amzd requested a review from r10s January 5, 2025 15:54
@Amzd Amzd marked this pull request as ready for review January 5, 2025 15:54
@r10s
Copy link
Member

r10s commented Jan 10, 2025

hm, i am not sure i got the gist of this PR completely.

  • swipe-down-to-exit-gallery worked also before, and at least on iOS 18, even smoother:

    • really only the image is swiped down, not the whole screen including black borders - this looks esp weird for landscape images
    • no 1-second-delay before the message view with the image is shown again
  • opening the gallery without any controls to share etc. may be unexpected: none of whatsapp or telegram do that. the controls only disappear on a second tap

  • the message view scrolling in-sync with the gallery left/right: not sure if that is expected by the user or not - whatsapp does it, telegram does not

  • the reduced motion is nice

however, always good to have options, so thanks for diving into that. and as said, sorry, if i missed sth.

@Amzd
Copy link
Collaborator Author

Amzd commented Jan 12, 2025

TODO: on iOS 18 the keyboard is hidden after viewing a photo, can be fixed by a GiveBackMyFirstResponder extension for UINavigationController.pushViewController(_: QLPreviewController) that does the same as UIViewController.present(_: QLPreviewController)

@Amzd
Copy link
Collaborator Author

Amzd commented Jan 13, 2025

Scratch that, that opened a massive can of worms. On iOS 18 the keyboard is just hidden when you click on an image in chat and it does not come back automatically, this is a fine UX I think (and not changed by this PR).

@Amzd
Copy link
Collaborator Author

Amzd commented Jan 21, 2025

Can take another look at iOS 18 having hidden keyboard after QLQuickLook if we want to merge #2536 because swizzling will probably have no negative impacts when pushing either.

@r10s
Copy link
Member

r10s commented Jan 25, 2025

this needs a rebase - is this PR still needed or conflicting with #2536 ?

@Amzd
Copy link
Collaborator Author

Amzd commented Jan 26, 2025

I will take another look at this after merge of the other PR

@Amzd
Copy link
Collaborator Author

Amzd commented Jan 29, 2025

Rebased and checked again for iOS 18 but couldn't get the "return keyboard after preview dismiss" to work with pushViewController (which we use because since iOS 18 when you present(_::) a QLPreviewController it doesn't show navigation and toolbars by default). (Note that this is already the behavior without this PR)

@r10s
Copy link
Member

r10s commented Jan 29, 2025

swipe-to-close is now indeed smooth again on iOS 18 (it was also smooth before), however, opening is worse:

for a ~second, a weird title bar is shown, with whitespace atop:

Screenshot 2025-01-29 at 15 55 19

video
ScreenRecording_01-29-2025.15-41-56_1.mov

it is not super-bad, but it looks as if something is wrong.

and, without this PR, the opening does not have the weird effect.

the effect is also there for landscape images, and the recording is done without debugger attached, so already "fast"

did not test iOS 15 yet

@r10s
Copy link
Member

r10s commented Jan 29, 2025

things got weirder: force-restarting the app, opening is fine :)

so maybe related to some system hickup.

so, in general, things look good on iOS 18

@zeitschlag as you're more into iOS, can you have a look at the code and test on your phones as well?

EDIT: i was on the wrong branch, so the issue from #2482 (comment) is reproducible, sorry for condusion

@r10s r10s requested review from zeitschlag and removed request for r10s and zeitschlag January 29, 2025 15:05
@r10s r10s marked this pull request as draft January 29, 2025 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement actually in development, user visible enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants