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 new type of alert — Loading Alert #20

Closed
wants to merge 2 commits into from

Conversation

mlouli
Copy link

@mlouli mlouli commented Mar 17, 2020

If you think it's a bad idea to add a loading option, then let's discuss it, maybe you should simply open some of the SPAlert.swift API so anyone using your project can subclass and add things easily.

Also, I noticed that when dark mode is enabled + multitasking on iPad, the blurred background is a little bit weird but I didn't find a solution yet.

Last but not least, why you prefered going with some frame calculation instead of using the auto-layout magic?

@ivanvorobei
Copy link
Member

@mlouli thanks for PR!
I fast check code and see many interesting changes. Let me full understand it and after merge.

About

dark mode is enabled + multitasking on iPad

You can show video or screenshoot?

@mlouli
Copy link
Author

mlouli commented Mar 31, 2020

Take your time for the PR, I am open for discussions if needed.

For dark mode issue when multitasking on iPad, I will try adding today a screenshot or sth.

Take care

@mlouli
Copy link
Author

mlouli commented Mar 31, 2020

RPReplay_Final1585656193.MP4.zip

I compressed the recording, don't hesitate to tell me if you prefer another format then ".zip"

There are weird lines behind the alert when multitasking as you can see in the screen-recording, I also tried with several types of alert, so it's not specific to one alert

@ivanvorobei
Copy link
Member

Thanks for video! But I not see any problem, you can explain more?

@mlouli
Copy link
Author

mlouli commented Mar 31, 2020

Screenshot 2020-03-31 at 22 03 02

Yeah, I agree that it's hard to see 😅.

I attached a screenshot, and I suggest replaying the record I sent you and try to focus plz on the background of the alert, you will notice some strange stains only when multitasking 🧐

Also if you can try to build the Example Project on an iPad (real or simulator), maybe you will see it easily 🤷‍♂️

@ivanvorobei
Copy link
Member

@mlouli catch!
I think it blur render problem

@ivanvorobei
Copy link
Member

@mlouli good day, I return and check code. Thanks for clean!

So, about loading. I sew two cases, which need attention:
We need use default struct with presets, lie this
SPAlert.presentLoading(message: "Loading...", preset: .loading)

And I think all users want close loading alert not with timeout. They prefer hide it by event.
By this case, when you have time, you can do you work as extension of current system?

If you need access to some internal properties or any other - please, write me.
Also I available in telegram @ivanvorobei

If you don't want do it, also please write me. I using your code add it as extension. For now I close PR, but I think your idea with loading is great. And wait info from you.

@ivanvorobei ivanvorobei closed this Apr 5, 2020
@ivanvorobei ivanvorobei mentioned this pull request Apr 5, 2020
@productdevbook
Copy link

new version how to used loading ?

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.

3 participants