[ios] Fix isolines reminder being displayed on top of other dialogs #7694

kirylkaveryn · 2024-03-26T00:40:44Z

v-lozko commented

2024-03-26 00:40:44 +00:00

(Migrated from github.com)

Fixing issue #7555 by adding capability to hide toast when view disappears. Also addressed potential other cases.

https://github.com/organicmaps/organicmaps/assets/156805389/b8c4d10a-56ca-4544-9417-61b4eab98f79

Tested on iPhone 15 simulator

Fixing issue #7555 by adding capability to hide toast when view disappears. Also addressed potential other cases. https://github.com/organicmaps/organicmaps/assets/156805389/b8c4d10a-56ca-4544-9417-61b4eab98f79 Tested on iPhone 15 simulator

biodranik (Migrated from github.com) approved these changes 2024-03-26 22:20:34 +00:00

biodranik (Migrated from github.com) left a comment

Great!

Please also check DCO. If you need to set a different email for OM, you can edit .git/config file in organicmaps repo.

Great! Please also check DCO. If you need to set a different email for OM, you can edit .git/config file in organicmaps repo.

biodranik (Migrated from github.com) reviewed 2024-03-27 21:25:42 +00:00

biodranik (Migrated from github.com) left a comment

Thanks! @kirylkaveryn WDYT?

kirylkaveryn reviewed 2024-03-30 10:38:20 +00:00

iphone/Maps/Classes/CustomAlert/Toast/Toast.swift

kirylkaveryn commented

2024-03-30 10:07:37 +00:00

It seems like the onTimer() method can be removed now and you can pass the hide in selector.

It seems like the `onTimer()` method can be removed now and you can pass the `hide` in selector.

iphone/Maps/Classes/CustomViews/MapViewControls/TrafficButton/MWMTrafficButtonViewController.mm

kirylkaveryn commented

2024-03-30 10:36:48 +00:00

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?

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: ```swift 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?

biodranik (Migrated from github.com) reviewed 2024-03-30 21:20:16 +00:00

iphone/Maps/Classes/CustomViews/MapViewControls/TrafficButton/MWMTrafficButtonViewController.mm

biodranik (Migrated from github.com) commented

2024-03-30 21:20:16 +00:00

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?

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?

kirylkaveryn reviewed 2024-03-31 05:32:34 +00:00

iphone/Maps/Classes/CustomViews/MapViewControls/TrafficButton/MWMTrafficButtonViewController.mm

kirylkaveryn commented

2024-03-31 05:32:34 +00:00

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.

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.

v-lozko (Migrated from github.com) reviewed 2024-04-02 12:59:24 +00:00

iphone/Maps/Classes/CustomViews/MapViewControls/TrafficButton/MWMTrafficButtonViewController.mm

v-lozko (Migrated from github.com) commented

2024-04-02 12:59:24 +00:00

So, what are the next steps regarding this issue? Would you like me to edit the commit and remove the onTimer method?

biodranik (Migrated from github.com) reviewed 2024-04-02 20:58:23 +00:00

iphone/Maps/Classes/CustomViews/MapViewControls/TrafficButton/MWMTrafficButtonViewController.mm

biodranik (Migrated from github.com) commented

2024-04-02 20:58:23 +00:00

@v-lozko is it hard to implement it in a way described by @kirylkaveryn ? What do you think about that approach?

v-lozko (Migrated from github.com) reviewed 2024-04-03 18:00:37 +00:00

iphone/Maps/Classes/CustomViews/MapViewControls/TrafficButton/MWMTrafficButtonViewController.mm

v-lozko (Migrated from github.com) commented

2024-04-03 18:00:36 +00:00

I can give it a go

v-lozko (Migrated from github.com) reviewed 2024-04-03 20:28:28 +00:00

iphone/Maps/Classes/CustomViews/MapViewControls/TrafficButton/MWMTrafficButtonViewController.mm

v-lozko (Migrated from github.com) commented

2024-04-03 20:28:28 +00:00

@biodranik , @kirylkaveryn I made the changes as suggested. Seems to work correctly

https://github.com/organicmaps/organicmaps/assets/156805389/e08717e5-3406-49a8-bc18-aa59e9646f2e

@biodranik , @kirylkaveryn I made the changes as suggested. Seems to work correctly https://github.com/organicmaps/organicmaps/assets/156805389/e08717e5-3406-49a8-bc18-aa59e9646f2e

biodranik (Migrated from github.com) reviewed 2024-04-04 22:09:59 +00:00

iphone/Maps/Classes/CustomAlert/Toast/Toast.swift

biodranik (Migrated from github.com) commented

2024-04-04 22:09:18 +00:00

Why a copy is needed?

kirylkaveryn reviewed 2024-04-05 07:29:41 +00:00

iphone/Maps/Classes/CustomAlert/Toast/Toast.swift

					
				@ -7,11 +7,20 @@ final class Toast: NSObject {

				  }

kirylkaveryn commented

2024-04-05 07:19:52 +00:00

  @objc static func hideAll() {

```suggestion @objc static func hideAll() { ```

kirylkaveryn commented

2024-04-05 07:29:37 +00:00

nit: it's better to write in single line easy iterations toastsCopy.forEach { $0.hide() }

nit: it's better to write in single line easy iterations `toastsCopy.forEach { $0.hide() }`

iphone/Maps/Classes/CustomAlert/Toast/Toast.swift

					
				@ -85,0 +97,4 @@

				    if self.blurView.superview != nil {

				      UIView.animate(withDuration: kDefaultAnimationDuration,

				                     animations: { self.blurView.alpha = 0 }) { [self] _ in

				        self.blurView.removeFromSuperview()