attachment instance id workaround

toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/Popup.kt

@@ -51,6 +51,7 @@ import com.arcgismaps.mapping.popup.AttachmentsPopupElement
 import com.arcgismaps.mapping.popup.FieldsPopupElement
 import com.arcgismaps.mapping.popup.MediaPopupElement
 import com.arcgismaps.mapping.popup.Popup
+import com.arcgismaps.mapping.popup.PopupAttachment
 import com.arcgismaps.mapping.popup.TextPopupElement
 import com.arcgismaps.toolkit.popup.internal.element.attachment.AttachmentsElementState
 import com.arcgismaps.toolkit.popup.internal.element.attachment.AttachmentsPopupElement
@@ -96,25 +97,43 @@ public fun Popup(popup: Popup, modifier: Modifier = Modifier) {
     Popup(stateData, modifier)
 }
 
+/**
+ * Maintain list of attachments outside of SDK
+ * https://devtopia.esri.com/runtime/apollo/issues/681
+ */
+private val attachments: MutableList<PopupAttachment> = mutableListOf()
+
 @Suppress("unused_parameter")
 @Composable
 private fun Popup(popupState: PopupState, modifier: Modifier = Modifier) {
     val popup = popupState.popup
     var evaluated by rememberSaveable(popup) { mutableStateOf(false) }
+    var fetched by rememberSaveable(popup) { mutableStateOf(false) }
 
     LaunchedEffect(popup) {
         popupState.popup.evaluateExpressions()
-        popupState.popup.evaluatedElements
-            .filterIsInstance<AttachmentsPopupElement>()
-            .firstOrNull()?.fetchAttachments()
+        if (!fetched) {
+            val element = popupState.popup.evaluatedElements
+                .filterIsInstance<AttachmentsPopupElement>()
+                .firstOrNull()
+
+            // make a copy of the attachments when first fetched.
+            attachments.clear()
+            element?.fetchAttachments()?.onSuccess {
+                attachments.addAll(element.attachments)
+            }
+
+            fetched = true
+        }
+
         evaluated = true
     }
 
-    Popup(popupState, evaluated)
+    Popup(popupState, evaluated && fetched)
 }
 
 @Composable
-private fun Popup(popupState: PopupState, evaluated: Boolean, modifier: Modifier = Modifier) {
+private fun Popup(popupState: PopupState, initialized: Boolean, modifier: Modifier = Modifier) {
     val scope = rememberCoroutineScope()
     val popup = popupState.popup
     val viewableFileState = rememberSaveable { mutableStateOf<ViewableFile?>(null) }
@@ -137,10 +156,10 @@ private fun Popup(popupState: PopupState, evaluated: Boolean, modifier: Modifier
                 .height(15.dp)
         )
         InitializingExpressions(modifier = Modifier.fillMaxWidth()) {
-            evaluated
+            initialized
         }
         HorizontalDivider(modifier = Modifier.fillMaxWidth(), thickness = 2.dp)
-        if (evaluated) {
+        if (initialized) {
             PopupBody(popupState) {
                 viewableFileState.value = it
             }
@@ -152,7 +171,7 @@ private fun Popup(popupState: PopupState, evaluated: Boolean, modifier: Modifier
 private fun PopupBody(popupState: PopupState, onFileClicked: (ViewableFile?) -> Unit = {}) {
     val popup = popupState.popup
     val lazyListState = rememberLazyListState()
-    val states = rememberStates(popup)
+    val states = rememberStates(popup, attachments)
     LazyColumn(
         modifier = Modifier
             .fillMaxSize()
@@ -232,7 +251,8 @@ internal fun InitializingExpressions(
  */
 @Composable
 internal fun rememberStates(
-    popup: Popup
+    popup: Popup,
+    attachments: List<PopupAttachment>
 ): PopupElementStateCollection {
     val states = mutablePopupElementStateCollection()
     popup.evaluatedElements.forEach { element ->
@@ -247,7 +267,11 @@ internal fun rememberStates(
             is AttachmentsPopupElement -> {
                 states.add(
                     element,
-                    rememberAttachmentsElementState(popup = popup, element = element)
+                    rememberAttachmentsElementState(
+                        popup = popup,
+                        element = element,
+                        attachments = attachments
+                    )
                 )
             }
 

toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/internal/element/attachment/AttachmentTile.kt

@@ -55,7 +55,6 @@ import androidx.compose.ui.text.TextStyle
 import androidx.compose.ui.text.font.FontWeight
 import androidx.compose.ui.text.style.TextAlign
 import androidx.compose.ui.text.style.TextOverflow
-import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.dp
 import androidx.compose.ui.unit.sp
 import coil.compose.AsyncImage
@@ -64,7 +63,6 @@ import com.arcgismaps.mapping.popup.PopupAttachmentType
 import com.arcgismaps.toolkit.popup.R
 import com.arcgismaps.toolkit.popup.internal.ui.fileviewer.ViewableFile
 import com.arcgismaps.toolkit.popup.internal.ui.fileviewer.getViewableFileType
-import kotlinx.coroutines.flow.MutableStateFlow
 
 @Composable
 internal fun AttachmentTile(
@@ -94,7 +92,7 @@ internal fun AttachmentTile(
                         ViewableFile(
                             state.name,
                             state.size,
-                            state.path,
+                            state.thumbnailUri.value,
                             state.getViewableFileType(),
                             state.contentType
                         )
@@ -273,18 +271,3 @@ private fun AttachmentSize(
             .padding(horizontal = 1.dp)
     )
 }
-
-@Preview(showBackground = true)
-@Composable
-internal fun PreviewAttachmentTile() {
-    AttachmentTile(
-        state = PopupAttachmentState(
-            name = "Some attachment",
-            size = 1234L,
-            popupAttachmentType = PopupAttachmentType.Other,
-            loadStatus = MutableStateFlow(LoadStatus.NotLoaded),
-            contentType = "image/jpeg",
-            onLoadAttachment = { Result.success(Unit) }
-        )
-    )
-}

toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/internal/element/attachment/AttachmentsElementState.kt

@@ -26,7 +26,6 @@ import androidx.compose.runtime.Immutable
 import androidx.compose.runtime.MutableState
 import androidx.compose.runtime.State
 import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.saveable.Saver
 import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.ui.graphics.vector.ImageVector
@@ -51,20 +50,22 @@ internal class AttachmentsElementState(
     override val id: Int = createId(),
 ) : PopupElementState() {
 
-    constructor(attachmentPopupElement: AttachmentsPopupElement) : this(
+    constructor(attachmentPopupElement: AttachmentsPopupElement, attachments: List<PopupAttachment>) : this(
         description = attachmentPopupElement.description,
         title = attachmentPopupElement.title,
-        attachments = attachmentPopupElement.attachments.map { PopupAttachmentState(it) }
+        attachments = attachments.map { PopupAttachmentState(it) }
     )
 
     companion object {
         fun Saver(
-            element: AttachmentsPopupElement
+            element: AttachmentsPopupElement,
+            attachments: List<PopupAttachment>
         ): Saver<AttachmentsElementState, Any> = Saver(
             save = { null },
             restore = {
                 AttachmentsElementState(
-                    element
+                    element,
+                    attachments
                 )
             }
         )
@@ -75,76 +76,51 @@ internal class AttachmentsElementState(
 @Composable
 internal fun rememberAttachmentsElementState(
     element: AttachmentsPopupElement,
-    popup: Popup
+    popup: Popup,
+    attachments: List<PopupAttachment>
 ): AttachmentsElementState {
-    val scope = rememberCoroutineScope()
     return rememberSaveable(
         inputs = arrayOf(popup, element),
-        saver = AttachmentsElementState.Saver(element)
+        saver = AttachmentsElementState.Saver(element, attachments)
     ) {
         AttachmentsElementState(
-            element
-        ).also {
-            // NOTE: core issue with PopupAttachments not abiding the instance id contract here.
-            // Loaded attachments are coming back NotLoaded after rotation.
-            // https://devtopia.esri.com/runtime/apollo/issues/681
-            it.attachments
-                .filter { state ->
-                    state.loadStatus.value == LoadStatus.Loaded
-                            && state.popupAttachmentType == PopupAttachmentType.Image
-                }
-                .forEach { state ->
-                    state.loadAttachment(scope)
-                }
-        }
+            element,
+            attachments
+        )
     }
 }
 
 /**
  * Represents the state of a [PopupAttachment].
  *
- * @param name The name of the attachment.
- * @param size The size of the attachment.
- * @param loadStatus The load status of the attachment.
- * @param onLoadAttachment A function that loads the attachment.
+ * @property attachment the popup attachment
  */
 internal class PopupAttachmentState(
-    val name: String,
-    val size: Long,
-    val popupAttachmentType: PopupAttachmentType,
-    val contentType: String,
-    val loadStatus: StateFlow<LoadStatus>,
-    private val onLoadAttachment: suspend () -> Result<Unit>,
+    private val attachment: PopupAttachment
 ) {
 
-    private lateinit var _attachment: PopupAttachment
+    val name: String = attachment.name
+    val size: Long = attachment.size
+    val popupAttachmentType: PopupAttachmentType = attachment.type
+    val contentType: String = attachment.contentType
+    val loadStatus: StateFlow<LoadStatus> = attachment.loadStatus
+
+    private val _thumbnailUri: MutableState<String> = mutableStateOf(attachment.filePath)
 
-    private val _thumbnailUri: MutableState<String> = mutableStateOf("")
     /**
      * The URI of the attachment image. Empty until [loadAttachment] is called.
      */
     val thumbnailUri: State<String> = _thumbnailUri
-    val path: String
-        get() = _attachment.filePath
-
-    constructor(attachment: PopupAttachment) : this(
-        name = attachment.name,
-        size = attachment.size,
-        popupAttachmentType = attachment.type,
-        contentType = attachment.contentType,
-        loadStatus = attachment.loadStatus,
-        onLoadAttachment = attachment::retryLoad
-    ) {
-        _attachment = attachment
-    }
 
     /**
      * Loads the attachment and its thumbnail.
      */
     fun loadAttachment(scope: CoroutineScope) {
-        scope.launch {
-            onLoadAttachment().onSuccess {
-                _thumbnailUri.value = _attachment.filePath
+        if (attachment.loadStatus.value !is LoadStatus.Loaded) {
+            scope.launch {
+                attachment.retryLoad().onSuccess {
+                    _thumbnailUri.value = attachment.filePath
+                }
             }
         }
     }

toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/internal/element/attachment/AttachmentsPopupElement.kt

@@ -25,13 +25,9 @@ import androidx.compose.foundation.lazy.items
 import androidx.compose.foundation.lazy.rememberLazyListState
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
-import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.dp
-import com.arcgismaps.LoadStatus
-import com.arcgismaps.mapping.popup.PopupAttachmentType
 import com.arcgismaps.toolkit.popup.internal.ui.ExpandableCard
 import com.arcgismaps.toolkit.popup.internal.ui.fileviewer.ViewableFile
-import kotlinx.coroutines.flow.MutableStateFlow
 
 @Composable
 internal fun AttachmentsPopupElement(
@@ -83,22 +79,3 @@ private fun AttachmentGallery(
         }
     }
 }
-
-@Preview
-@Composable
-private fun AttachmentsPopupElementPreview() {
-    AttachmentsPopupElement(
-        title = "Attachments",
-        description = "description of attachments",
-        stateId = 1,
-        attachments = listOf(
-            PopupAttachmentState(
-                name = "Photo 1.jpg",
-                size = 2024,
-                PopupAttachmentType.Image,
-                contentType = "image/jpeg",
-                MutableStateFlow(LoadStatus.Loaded)
-            ) { Result.success(Unit) }
-        )
-    )
-}