Clean up ITMActionable classes (#79)

mobile-sdk/src/main/java/com/github/itwin/mobilesdk/Extensions.kt

@@ -85,22 +85,31 @@ fun <K, V> Map<K, V>.getOptionalDouble(key: K) =
  * @return The receiver specialized with `K` and `V` if the key and value types match, otherwise
  * `null`.
  */
-inline fun <reified K: Any, reified V: Any> Map<*,*>.checkEntriesAre(): Map<K, V>? {
-    forEach {
-        if (it.key !is K) return null
-        if (it.value !is V) return null
+inline fun <reified K: Any, reified V: Any> Map<*,*>.checkEntriesAre(): Map<K, V>? =
+    this.takeIf {
+        all { it.key is K && it.value is V }
+    }?.let {
+        @Suppress("UNCHECKED_CAST")
+        it as Map<K, V>
     }
-    @Suppress("UNCHECKED_CAST")
-    return this as Map<K, V>
-}
+
+/**
+ * Ensure that all entries in the receiver have a key type of `K` and a value type of `V`.
+ * @throws Throwable If the entries don't have the proper types, this throws while failing to convert
+ * `null` to `Map<K, V>`.
+ * @return The receiver specialized with `K` and `V`.
+ */
+inline fun <reified K: Any, reified V: Any> Map<*,*>.ensureEntriesAre(): Map<K, V> =
+    checkEntriesAre<K, V>() as Map<K, V>
 
 /**
  * Verify that all items in the receiver have a type of `T`.
  * @return The receiver specialized with `T` if the item types match, otherwise `null`.
  */
 inline fun <reified T> List<*>.checkItemsAre(): List<T>? =
-    if (all { it is T })
+    this.takeIf {
+        all { it is T }
+    }?.let {
         @Suppress("UNCHECKED_CAST")
-        this as List<T>
-    else
-        null
+        it as List<T>
+    }

mobile-sdk/src/main/java/com/github/itwin/mobilesdk/ITMActionSheet.kt

@@ -19,7 +19,7 @@ import kotlin.coroutines.suspendCoroutine
  *
  * This class is used by the `presentActionSheet` TypeScript function in `@itwin/mobile-core`.
  *
- * __Note:__ Due to the cross-platform nature of `@itwin/mobile-core`, functionality like this that
+ * > __Note:__ Due to the cross-platform nature of `@itwin/mobile-core`, functionality like this that
  * is designed to use native underlying features runs into a possible confusion over different naming
  * on iOS vs. Android. On iOS, the `presentActionSheet` TypeScript function results in an alert
  * controller with a style of `actionSheet`. Functionally, that uses a popover on iPads and full screen
@@ -40,6 +40,18 @@ class ITMActionSheet(nativeUI: ITMNativeUI): ITMActionable(nativeUI) {
         handler = coMessenger.registerQueryHandler("Bentley_ITM_presentActionSheet", ::handleQuery)
     }
 
+    private fun toGravity(value: String?) = when (value) {
+        "top" -> Gravity.TOP
+        "bottom" -> Gravity.BOTTOM
+        "left" -> Gravity.LEFT
+        "right" -> Gravity.RIGHT
+        "topLeft" -> Gravity.TOP or Gravity.LEFT
+        "topRight" -> Gravity.TOP or Gravity.RIGHT
+        "bottomLeft" -> Gravity.BOTTOM or Gravity.LEFT
+        "bottomRight" -> Gravity.BOTTOM or Gravity.RIGHT
+        else -> Gravity.NO_GRAVITY
+    }
+
     private suspend fun handleQuery(params: Map<String, Any>): String? {
         try {
             // Note: no input validation is intentional. If the input is malformed, it will trigger the exception handler, which will send
@@ -52,41 +64,39 @@ class ITMActionSheet(nativeUI: ITMNativeUI): ITMActionable(nativeUI) {
             addAnchor(ITMRect(params["sourceRect"] as Map<*, *>, webView))
             return suspendCoroutine { continuation ->
                 this.continuation = continuation
-                val popupGravity = params.getOptionalString("gravity")?.toGravity() ?: Gravity.NO_GRAVITY
+                val popupGravity = toGravity(params.getOptionalString("gravity"))
                 with(PopupMenu(context, anchor, popupGravity)) {
-                    setOnMenuItemClickListener { item ->
+                    popupMenu = this
+                    setOnMenuItemClickListener {
                         removeAnchor()
                         popupMenu = null
-                        cancelAction = null
-                        resume(actions[item.itemId].name)
+                        resume(actions[it.itemId].name)
                         return@setOnMenuItemClickListener true
                     }
                     setOnDismissListener {
                         removeAnchor()
                         popupMenu = null
                         resume(cancelAction?.name)
                     }
-                    params.getOptionalString("title")?.let { title ->
-                        with(menu.add(Menu.NONE, -1, Menu.NONE, title)) {
+                    params.getOptionalString("title")?.let {
+                        with(menu.add(Menu.NONE, -1, Menu.NONE, it)) {
                             isEnabled = false
                         }
                     }
-                    params.getOptionalString("message")?.let { message ->
-                        with(menu.add(Menu.NONE, -1, Menu.NONE, message)) {
+                    params.getOptionalString("message")?.let {
+                        with(menu.add(Menu.NONE, -1, Menu.NONE, it)) {
                             isEnabled = false
                         }
                     }
                     for ((index, action) in actions.withIndex()) {
                         menu.add(Menu.NONE, index, Menu.NONE, action.styledTitle)
                     }
                     show()
-                    popupMenu = this
                 }
             }
         } catch (ex: Exception) {
             removeUI()
             removeAnchor()
-            cancelAction = null
             continuation = null
             // Note: this is caught by ITMCoMessenger and tells the TypeScript caller that there was an error.
             throw Exception("Invalid input to Bentley_ITM_presentActionSheet")
@@ -96,6 +106,7 @@ class ITMActionSheet(nativeUI: ITMNativeUI): ITMActionable(nativeUI) {
     override fun removeUI() {
         popupMenu?.dismiss()
         popupMenu = null
+        cancelAction = null
     }
 
     private fun addAnchor(sourceRect: ITMRect) {
@@ -118,12 +129,14 @@ class ITMActionSheet(nativeUI: ITMNativeUI): ITMActionable(nativeUI) {
                 val (x, y) = webView.screenLocation()
                 layoutParams.leftMargin += x
                 layoutParams.topMargin += y
+                // Add the anchor to relativeLayout
                 addView(anchor, layoutParams)
                 val matchParent = RelativeLayout.LayoutParams.MATCH_PARENT
                 val screenLayoutParams = RelativeLayout.LayoutParams(matchParent, matchParent)
                 screenLayoutParams.addRule(RelativeLayout.ALIGN_PARENT_TOP, RelativeLayout.TRUE)
                 screenLayoutParams.addRule(RelativeLayout.ALIGN_PARENT_LEFT, RelativeLayout.TRUE)
                 viewGroup = viewGroup?.rootView as? ViewGroup
+                // Add the full-screen relativeLayout to viewGroup
                 viewGroup?.addView(this, screenLayoutParams)
             }
         } else {
@@ -137,32 +150,18 @@ class ITMActionSheet(nativeUI: ITMNativeUI): ITMActionable(nativeUI) {
         }
         relativeLayout = null
         anchor = null
+        viewGroup = null
     }
 
     /**
      * Cancels the action sheet when the device configuration changes (for example during an orientation change).
      */
     override fun onConfigurationChanged(newConfig: Configuration) {
         super.onConfigurationChanged(newConfig)
-        removeUI()
-        removeAnchor()
         resume(cancelAction?.name)
-        cancelAction = null
     }
 }
 
-private fun String.toGravity() = when (this) {
-    "top" -> Gravity.TOP
-    "bottom" -> Gravity.BOTTOM
-    "left" -> Gravity.LEFT
-    "right" -> Gravity.RIGHT
-    "topLeft" -> Gravity.TOP or Gravity.LEFT
-    "topRight" -> Gravity.TOP or Gravity.RIGHT
-    "bottomLeft" -> Gravity.BOTTOM or Gravity.LEFT
-    "bottomRight" -> Gravity.BOTTOM or Gravity.RIGHT
-    else -> Gravity.NO_GRAVITY
-}
-
 private fun View.screenLocation(): IntArray {
     val location = IntArray(2)
     getLocationOnScreen(location)

mobile-sdk/src/main/java/com/github/itwin/mobilesdk/ITMActionable.kt

@@ -24,10 +24,14 @@ abstract class ITMActionable(nativeUI: ITMNativeUI): ITMNativeUIComponent(native
          * @param tsActions A List of [Map] values containing the actions.
          */
         fun readActions(tsActions: List<*>): Pair<List<Action>, Action?> {
+            // Note: Various things here can trigger an exception if invalid input is received. We
+            // intentionally allow those to happen so that this function will throw an exception if
+            // there is invalid input, which will then be caught by the caller, which generates an
+            // error log and returns the exception to the TypeScript side.
             val actions: MutableList<Action> = mutableListOf()
             var cancelAction: Action? = null
             tsActions.forEach { actionValue ->
-                (actionValue as? Map<*, *>)?.checkEntriesAre<String, String>()?.let {
+                (actionValue as Map<*, *>).ensureEntriesAre<String, String>().let {
                     val action = Action(it)
                     if (action.style == Action.Style.Cancel) {
                         cancelAction = action
@@ -50,7 +54,12 @@ abstract class ITMActionable(nativeUI: ITMNativeUI): ITMNativeUIComponent(native
             Destructive;
 
             companion object {
-                fun fromString(style: String?) = style?.takeIf { it.isNotEmpty() }?.let { Style.valueOf(style.replaceFirstChar { it.uppercase() }) } ?: Default
+                fun fromString(style: String?) =
+                    style?.takeIf {
+                        it.isNotEmpty()
+                    }?.let {
+                        Style.valueOf(style.replaceFirstChar { it.uppercase() })
+                    } ?: Default
             }
         }
 
@@ -65,12 +74,12 @@ abstract class ITMActionable(nativeUI: ITMNativeUI): ITMNativeUIComponent(native
          */
         val styledTitle: CharSequence
             get() = if (style == Action.Style.Destructive) {
-                    val colorTitle = SpannableString(title)
-                    colorTitle.setSpan(ForegroundColorSpan(Color.RED), 0, title.length, 0)
-                    colorTitle
-                } else {
-                    title
+                SpannableString(title).apply {
+                    setSpan(ForegroundColorSpan(Color.RED), 0, title.length, 0)
                 }
+            } else {
+                title
+            }
     }
 
     /**

mobile-sdk/src/main/java/com/github/itwin/mobilesdk/ITMAlert.kt

@@ -5,7 +5,6 @@
 package com.github.itwin.mobilesdk
 
 import android.app.AlertDialog
-import java.util.*
 import kotlin.coroutines.suspendCoroutine
 
 /**
@@ -22,7 +21,34 @@ class ITMAlert(nativeUI: ITMNativeUI): ITMActionable(nativeUI)  {
         handler = coMessenger.registerQueryHandler("Bentley_ITM_presentAlert", ::handleQuery)
     }
 
-    @Suppress("LongMethod")
+    private fun toAlertActionsOrItems(actions: List<Action>): Pair<Array<Action?>, MutableList<CharSequence>?> {
+        var index = 0
+        var neutralAction: Action? = null
+        var negativeAction: Action? = null
+        var positiveAction: Action? = null
+        var items: MutableList<CharSequence>? = null
+        if (actions.size > 3) {
+            items = mutableListOf()
+            actions.forEach {
+                items += it.styledTitle
+            }
+        } else {
+            // Note: The mapping of actions to buttons is documented in mobile-sdk-core.
+            if (actions.size == 3) {
+                neutralAction = actions[index]
+                ++index
+            }
+            if (actions.size >= 2) {
+                negativeAction = actions[index]
+                ++index
+            }
+            if (actions.isNotEmpty()) {
+                positiveAction = actions[index]
+            }
+        }
+        return Pair(arrayOf(neutralAction, negativeAction, positiveAction), items)
+    }
+
     private suspend fun handleQuery(params: Map<String, *>): String? {
         try {
             // Note: no input validation is intentional. If the input is malformed, it will trigger the exception handler, which will send
@@ -31,30 +57,8 @@ class ITMAlert(nativeUI: ITMNativeUI): ITMActionable(nativeUI)  {
             val title = params.getOptionalString("title")
             val message = params.getOptionalString("message")
             if (actions.isEmpty() && cancelAction == null) throw Exception("No actions")
-            var index = 0
-            var neutralAction: Action? = null
-            var negativeAction: Action? = null
-            var positiveAction: Action? = null
-            var items: MutableList<CharSequence>? = null
-            if (actions.size > 3) {
-                items = mutableListOf()
-                actions.forEach { action ->
-                    items += action.styledTitle
-                }
-            } else {
-                // Note: The mapping of actions to buttons is documented in mobile-sdk-core.
-                if (actions.size == 3) {
-                    neutralAction = actions[index]
-                    ++index
-                }
-                if (actions.size >= 2) {
-                    negativeAction = actions[index]
-                    ++index
-                }
-                if (actions.isNotEmpty()) {
-                    positiveAction = actions[index]
-                }
-            }
+            val (alertActions, items) = toAlertActionsOrItems(actions)
+            val (neutralAction, negativeAction, positiveAction) = alertActions
             // If there is already an alert active, cancel it.
             resume(null)
             return suspendCoroutine { continuation ->