search

pmusolino / PMAlertController

PMAlertController is a great and customizable alert that can substitute UIAlertController
MIT License
2.53k stars 187 forks source link

Cannot scroll #42

Open pbmorton opened 7 years ago

pbmorton commented 7 years ago

If the alert text cannot fit on the screen the text cannot be scrolled - UIAlertController in contrast supports scrolling by default. What's worse is that the PMAlertController places the "cancel" action off the screen so a user cannot then cancel the alert. The App is effectively blocked from user interaction.

Being able to handle an alert that wont fit completely on screen is important. With devices with different configurations (e.g. screen sizes, portrait, landscape) and dynamic type sizes (i.e. through the settings App a user can increase the text size) its easy to imagine some alerts not fitting a screen.

Otherwise a great library thanks!

pmusolino commented 7 years ago

Hi @pbmorton, great observation. I marked this issue like enhancement, i can implement this feature in a future release.

Thanks, Paolo

pbmorton commented 7 years ago

Thanks Paolo. Can you estimate when this feature may be supported?

pmusolino commented 7 years ago

Asap, I'll try this week ;)

javalnanda commented 7 years ago

@Codeido Have applied the quick workaround for the scroll issue. Do check if it looks good to you https://github.com/Codeido/PMAlertController/pull/45

pmusolino commented 7 years ago

@javalnanda thanks for this PR, but i think that your workaround is not a final solution. First of all, if you substitute the label description with a textview you break the back compatibility. This feature need to be an enhancement. 2nd: this solution include only some cases, because if you have a lot of buttons, you have a small textview for the description (probably with height == 0).

javalnanda commented 7 years ago

@Codeido Makes sense, the back compatibility will be a major issue. Are you thinking of embedding entire alert view inside a scroll view?

pmusolino commented 7 years ago

Yes, i think that is the best solution.

javalnanda commented 7 years ago

ok, 👍 Let me know if you are already working on it. Else, I could spend some time on it the coming week.

javalnanda commented 7 years ago

@Codeido Scrolling entire alert view looks really bad. Just tried out. The Default AlertController also scrolls only the text part. I think we will need to scroll only the content part except Alert Action Stack View

javalnanda commented 7 years ago

@Codeido Sent one more PR, see if it is helpful. Also, I think it will be good to have a limit to the number of action buttons to max 5 or 6 maybe. Just a suggestion :)

edwardmp commented 6 years ago

Hi @Codeido

thanks for this nice alert controller! Wondering if you ever got around the merging in a fix for this?

thatmarcel commented 6 years ago

@pmusolino When will the feature be implemented?

sm0nster commented 4 years ago

It's still unscrollable, right?