skywinder / ActionSheetPicker-3.0

Quickly reproduce the dropdown UIPickerView / ActionSheet functionality on iOS.
http://skywinder.github.io/ActionSheetPicker-3.0
BSD 3-Clause "New" or "Revised" License
3.4k stars 740 forks source link

[BUG] - HIGH - Not working on screen rotation + Not working in Landscape #460

Closed StefaniOSApps closed 4 years ago

StefaniOSApps commented 4 years ago

Unfortunately I had to find out that the layout is destroyed when rotating under iOS 13. I have already checked it under 2.4.1 (from 2017) and also checked with develop branch with #379 . This error also exists with this version or with the develop branch.

I created a demo git so that you can easily reproduce it. https://github.com/StefaniOSApps/ActionSheet-Example

The error is shown in this video. Screen Record

StefaniOSApps commented 4 years ago

@skywinder

skywinder commented 4 years ago

@StefaniOSApps Oh, got it. thanks for the mention and detailed description!

Rotations screen was always quite painfull for dynamic views. I thing the best way to quick fix now - is just hide picker when the screen is rotating. Thougs?

StefaniOSApps commented 4 years ago

This will not fix the problem. If you open the sheet in the landscape, it will already be extended incorrectly. This is shown in the GIF. @skywinder

GIF SCREEN
StefaniOSApps commented 4 years ago

It will most likely be a method that is deprecated as of iOS 13.

skywinder commented 4 years ago

Can't realize, hot ti fix it in the right way. Looking forward to any suggestions.

StefaniOSApps commented 4 years ago

Hey, we need support. Any ideas? @xjki @BubiDevs @TimCinel, @kashifhisam or @delackner?

Jack-s commented 4 years ago

If it helps it looks like this is only an issue from the 2.4 release (2.3.1 seems to be fine). It also is an issue on iOS 12 as well so I don't think it's a iOS 13 specific issue.

StefaniOSApps commented 4 years ago

yes, it is most likely a bug generated by an iOS update.

skywinder commented 4 years ago

@Jack-s confirmed, in 2.3.1 it works. (it's simply dismisse picker)

@StefaniOSApps all that I can do now: to make an action, to dismiss the picker upon rotation. In most cases, it wouldn't be a problem. And I will preserve boolean trigger to switch off that behavior (with the warning, that developer do this at his own risk and it's not tested).

Thoughts?

StefaniOSApps commented 4 years ago

I don't think you have yet understood that the picker no longer works in the landscape. This is the main problem. You can also dismiss a picker by using your own methods. ezgif-6-ac2350cea71e

see demo project https://github.com/StefaniOSApps/ActionSheet-Example @skywinder

StefaniOSApps commented 4 years ago

btw ... the video above was made with 2.3.1 @Jack-s @skywinder

Downloading dependencies
Installing ActionSheetPicker-3.0 2.3.1 (was 2.4.1 and source changed to `https://cdn.cocoapods.org/` from `trunk`)
Generating Pods project
Integrating client project
Pod installation complete! There is 1 dependency from the Podfile and 1 total pod installed.
skywinder commented 4 years ago

oh, crap. @StefaniOSApps I'm digging into it.

I tried to run on 2.3.1 - it works fine on ios 11 and ios 13. So will go deep into diff now.

ps. your project (https://github.com/StefaniOSApps/ActionSheet-Example) doesn't specify version: look at podfile: pod 'ActionSheetPicker-3.0'

so you still use the latest one.

p.s. hope your Energy Tracker doesn't affect too much. will do my best right now.

skywinder commented 4 years ago

Gotcha. @moheny changes brokes rotation logic. now it works. But I have to dismiss picker on iPhones during rotation. (for iPad it works as it was before)

skywinder commented 4 years ago

Fixed in 2.4.2 🎉

skywinder commented 4 years ago

rotation fixes in ACP looks like.. 😂 https://imgur.com/pdGY72I

StefaniOSApps commented 4 years ago

@skywinder the commit (change podfile) was not pushed. I have now pushed the changes (commit) I have now extensively tested it and it fixes the problem on 13.0. However, the Dismiss rotation is broken. We currently differentiate between iPad true and false. In the case of iPad == false, one could revise and solve with a transition delegate. I have now implemented this in my new project regardless of you in this way and it looks good. The rotation also looks clean.

skywinder commented 4 years ago

@StefaniOSApps Just dun your example. looks good.

What you mean, "Dismiss rotation is broken"? How does it look like? On iphone? Which position? Can you provide any steps to reproduce?

StefaniOSApps commented 4 years ago

use demo project from top. @skywinder Ohne-Titel-2

skywinder commented 4 years ago

oh, got it. in slow motion, I can see this. I will leave it as is for a while.

so, please let me know, if you know, how to solve this.

skywinder commented 4 years ago

Oh. I found that didn't reope it. @StefaniOSApps Does it persist in latest versions?