PR feedback - moved action logs to Alerts.kt after handler and added …

…constructor to RestrictedLogger.kt so it can be called from SwiftUi without passing a logger.
splendo · Oct 17, 2023 · d6d1913 · d6d1913
1 parent e3ebdc8
commit d6d1913
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 21 deletions.
diff --git a/alerts/src/androidLibMain/kotlin/AlertPresenter.kt b/alerts/src/androidLibMain/kotlin/AlertPresenter.kt
@@ -31,7 +31,6 @@ import com.splendo.kaluga.base.utils.applyIf
 import com.splendo.kaluga.logging.Logger
 import com.splendo.kaluga.logging.RestrictedLogLevel
 import com.splendo.kaluga.logging.RestrictedLogger
-import com.splendo.kaluga.logging.info
 import com.splendo.kaluga.resources.dpToPixel
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.MutableStateFlow
@@ -118,12 +117,11 @@ actual class AlertPresenter(
         presentation.value = DialogPresentation.Hidden
     }
 
-    override fun showAlert(
+    override fun handleShowAlert(
         animated: Boolean,
         afterHandler: (Alert.Action?) -> Unit,
         completion: () -> Unit,
     ) {
-        super.showAlert(animated, afterHandler, completion)
         presentation.value = DialogPresentation.Showing(animated, afterHandler, completion)
     }
 
@@ -135,7 +133,6 @@ actual class AlertPresenter(
                 val titles = alert.actions.map { it.title }.toTypedArray()
                 setItems(titles) { _, which ->
                     val action = alert.actions[which].apply {
-                        logger.info(TAG, "Action ${this.title} was called on dialog with title: ${alert.title}")
                         handler()
                     }
                     presentation.afterHandler(action)
@@ -152,7 +149,6 @@ actual class AlertPresenter(
                 }
                 alert.actions.forEach { action ->
                     setButton(transform(action.style), action.title) { _, _ ->
-                        logger.info(TAG, "Action ${action.title} was called on dialog with title: ${alert.title}")
                         action.handler()
                         presentation.afterHandler(action)
                     }
@@ -161,7 +157,6 @@ actual class AlertPresenter(
             .applyIf(alert.style == Alert.Style.ALERT) {
                 alert.actions.forEach { action ->
                     setButton(transform(action.style), action.title) { _, _ ->
-                        logger.info(TAG, "Action ${action.title} was called on dialog with title: ${alert.title}")
                         action.handler()
                         presentation.afterHandler(action)
                     }
@@ -173,7 +168,6 @@ actual class AlertPresenter(
                     this@AlertPresenter.presentation.value = DialogPresentation.Hidden
                 }
                 setOnCancelListener {
-                    logger.info(TAG, "Canceling alert dialog with title: ${alert.title}")
                     presentation.afterHandler(null)
                 }
                 show()

diff --git a/alerts/src/commonMain/kotlin/Alerts.kt b/alerts/src/commonMain/kotlin/Alerts.kt
@@ -334,6 +334,10 @@ abstract class BaseAlertPresenter(private val alert: Alert, private val logger:
             showAlert(
                 animated,
                 afterHandler = { action ->
+                    // Null action is passed on cancel for both platforms
+                    action?.let {
+                        logger.info(TAG, "Action ${it.title} was called on dialog with title: ${alert.title}")
+                    } ?: logger.info(TAG, "Action Cancel was called on dialog with title: ${alert.title}")
                     continuation.tryResume(action)?.let {
                         continuation.completeResume(it)
                     }
@@ -349,13 +353,20 @@ abstract class BaseAlertPresenter(private val alert: Alert, private val logger:
         logger.info(TAG, "Dismissing alert dialog with title: ${alert.title}")
     }
 
-    protected open fun showAlert(
+    private fun showAlert(
         animated: Boolean = true,
         afterHandler: (Alert.Action?) -> Unit = {},
         completion: () -> Unit = {},
     ) {
         logger.info(TAG, "Displaying alert dialog with title: ${alert.title}")
+        handleShowAlert(animated, afterHandler, completion)
     }
+
+    protected abstract fun handleShowAlert(
+        animated: Boolean = true,
+        afterHandler: (Alert.Action?) -> Unit = {},
+        completion: () -> Unit = {},
+    )
 }
 
 /**

diff --git a/alerts/src/iosMain/kotlin/AlertPresenter.kt b/alerts/src/iosMain/kotlin/AlertPresenter.kt
@@ -21,7 +21,6 @@ package com.splendo.kaluga.alerts
 import com.splendo.kaluga.logging.Logger
 import com.splendo.kaluga.logging.RestrictedLogLevel
 import com.splendo.kaluga.logging.RestrictedLogger
-import com.splendo.kaluga.logging.info
 import kotlinx.cinterop.ObjCAction
 import kotlinx.coroutines.CoroutineScope
 import platform.Foundation.NSString
@@ -112,12 +111,11 @@ actual class AlertPresenter(
         parent.dismissModalViewControllerAnimated(animated)
     }
 
-    override fun showAlert(
+    override fun handleShowAlert(
         animated: Boolean,
         afterHandler: (Alert.Action?) -> Unit,
         completion: () -> Unit,
     ) {
-        super.showAlert(animated, afterHandler, completion)
         UIAlertController.alertControllerWithTitle(
             alert.title,
             alert.message,
@@ -131,7 +129,6 @@ actual class AlertPresenter(
                         action.title,
                         transform(action.style),
                     ) {
-                        logger.info(TAG, "Action ${action.title} was called on dialog with title: ${alert.title}")
                         action.handler()
                         afterHandler(action)
                     },
@@ -147,7 +144,6 @@ actual class AlertPresenter(
                         NSString.localizedStringWithFormat("Cancel"),
                         UIAlertActionStyleCancel,
                     ) {
-                        logger.info(TAG, "Action Cancel was called on dialog with title: ${alert.title}")
                         afterHandler(null)
                     },
                 )

diff --git a/alerts/src/jsMain/kotlin/AlertPresenter.kt b/alerts/src/jsMain/kotlin/AlertPresenter.kt
@@ -68,11 +68,7 @@ actual class AlertPresenter(
         TODO("not implemented")
     }
 
-    override fun showAlert(
-        animated: Boolean,
-        afterHandler: (Alert.Action?) -> Unit,
-        completion: () -> Unit,
-    ) {
-        TODO("not implemented")
+    override fun handleShowAlert(animated: Boolean, afterHandler: (Alert.Action?) -> Unit, completion: () -> Unit) {
+        TODO("Not yet implemented")
     }
 }
diff --git a/alerts/src/jvmMain/kotlin/AlertPresenter.kt b/alerts/src/jvmMain/kotlin/AlertPresenter.kt
@@ -30,7 +30,7 @@ import kotlinx.coroutines.CoroutineScope
  */
 actual class AlertPresenter(
     alert: Alert,
-    logger: Logger
+    logger: Logger,
 ) : BaseAlertPresenter(alert, logger) {
 
     /**
@@ -66,7 +66,7 @@ actual class AlertPresenter(
         TODO("not implemented")
     }
 
-    override fun showAlert(
+    override fun handleShowAlert(
         animated: Boolean,
         afterHandler: (Alert.Action?) -> Unit,
         completion: () -> Unit,

diff --git a/logging/src/commonMain/kotlin/RestrictedLogger.kt b/logging/src/commonMain/kotlin/RestrictedLogger.kt
@@ -72,6 +72,10 @@ infix fun RestrictedLogLevel.or(other: RestrictedLogLevel): RestrictedLogLevel =
  */
 class RestrictedLogger(private val restrictedLogLevel: RestrictedLogLevel, private val logger: Logger = defaultLogger) : Logger {
 
+    constructor(
+        restrictedLogLevel: RestrictedLogLevel
+    ) : this(restrictedLogLevel, defaultLogger)
+
     override fun log(
         level: LogLevel,
         tag: String?,