[ios] Fix isolines reminder being displayed on top of other dialogs #7694
Conversation
…Issue organicmaps#7555 Signed-off-by: Valery Lozko <valerylozko@gmail.com>
|
I amended the commit and looks like DCO now passes, sorry about that. |
|
@kirylkaveryn thanks! Can you please also review this PR? I think you replied to a comment from another issue/PR ) |
| repeats: false) | ||
| timer = Timer.scheduledTimer(timeInterval: 3, | ||
| target: self, | ||
| selector: #selector(onTimer), |
There was a problem hiding this comment.
It seems like the onTimer() method can be removed now and you can pass the hide in selector.
| @@ -36,6 +36,7 @@ @interface MWMTrafficButtonViewController () <MWMMapOverlayManagerObserver, Them | |||
| @property(nonatomic) NSLayoutConstraint *topOffset; | |||
| @property(nonatomic) NSLayoutConstraint *leftOffset; | |||
| @property(nonatomic) CGRect availableArea; | |||
| @property(nonatomic) MWMToast *toast; | |||
There was a problem hiding this comment.
I'm not sure about this approach... to store toast on every controller that we have in the project (where it needed of cours). And the other issue can happen: sometimes we can have 2-3 toasts at the same time (not in the current place, but in the PlacePage) and when we call hide() manually we will close only the last toast stored in the property.
Maybe it would be better to incapsulate toast logic inside of Toast class and store toasts in the static property?
With this approach we can hide all toasts from every place we want.
Some kine of:
class Toast {
// ...
static private var toasts: [Toast] = []
static func hideAll() {
toasts.forEach { $0.hide }
}
func show(...) {
Toast.appent(toast)
// ... showing code ...
}
func hide(...) {
//... hiding code ...
Toast.remove(toast)
}
@biodranik what do you think?
There was a problem hiding this comment.
In the long term, it would be great if users could read all toasts without overlapping each other if they appear simultaneously (e.g. stack them, or show one after another). A simpler way to control toast visibility is also great.
How does Apple solve toast issue?
There was a problem hiding this comment.
Apple doesn't have toasts at all. Only push notifications.
This is why toasts are so different from an app to app.
Storing all toasts logic inside the Toast class helps us in the future to handle the toasts queue properly with animations and gesture recognizers.
There was a problem hiding this comment.
So, what are the next steps regarding this issue? Would you like me to edit the commit and remove the onTimer method?
There was a problem hiding this comment.
@v-lozko is it hard to implement it in a way described by @kirylkaveryn ? What do you think about that approach?
There was a problem hiding this comment.
@biodranik , @kirylkaveryn I made the changes as suggested. Seems to work correctly
Simulator.Screen.Recording.-.iPhone.13.Pro.-.2024-04-03.at.16.27.26.mp4
Signed-off-by: Valery Lozko <valerylozko@gmail.com>
|
|
||
|
|
||
| @objc static func hideAll(){ | ||
| var toastsCopy = toasts |
There was a problem hiding this comment.
Was afraid of mutating the array while iterating over it. Doing a little looking it seems like it doesn't matter so i can make the change
| } | ||
|
|
||
|
|
||
| @objc static func hideAll(){ |
There was a problem hiding this comment.
| @objc static func hideAll(){ | |
| @objc static func hideAll() { |
| @objc private func onTimer() { | ||
| hide() | ||
|
|
||
| Self.toasts.removeAll(where: { $0 === self }) |
There was a problem hiding this comment.
I think it's better to try to place this line into the UIView.animate's completion handler to remove the toast instance as a final step.
| toastsCopy.forEach { | ||
| $0.hide() | ||
| } |
There was a problem hiding this comment.
nit: it's better to write in single line easy iterations toastsCopy.forEach { $0.hide() }
Jean-BaptisteC
changed the title
Signed-off-by: Valery Lozko <valerylozko@gmail.com>
|
Thanks for the review and comments. I made the changes |
Fixing issue #7555 by adding capability to hide toast when view disappears. Also addressed potential other cases.
Simulator.Screen.Recording.-.iPhone.15.-.2024-03-25.at.18.10.04.mp4
Tested on iPhone 15 simulator