Add support for UNAssociationsPopupElement to Popup #981

puneet-pdx · 2025-09-10T19:53:38Z

Related to issue: # https://devtopia.esri.com/runtime/apollo/issues/1407

Description:

Add support for UNAssociations in Popups.

Summary of changes:

Add PopupState, Add new Popup Composable that takes the State object
Update all internal popup elements to not have remember functions and just initialize them using constructors and hold them in the PopupState object
Add UtilityAssociationElement(from FeatureForms)
Add support for Navigation to enable browsing from one Popup to another based on an association element(from FeatureForms)
Add PopupScreen
Add ContentAwareTopBar(from FeatureForms)
Deprecate stateful PopupComposable
Update MicroApp to use the new Composable

Pre-merge Checklist

a vTest Job for this PR has been run
- link: https://runtime-kotlin.esri.com/view/all/job/vtest/job/toolkit/937/
- https://runtime-kotlin.esri.com/view/all/job/vtest/job/toolkit/946/
Unit and/or integration tests have been added to exercise this PR's logic, and the tests are passing:
- Yes
- No

remove support for hasEdits from contentAwareTopbar

replace form references with popup remove caching attachments in popupstate

add doc

deprecate popup composable

disable lint for missing translations

# Conflicts: # toolkit/popup/api/popup.api

update API file

kaushikrw

@puneet-pdx I wasn't able to load any map to test in the micro-app but here is some initial feedback.

...PopupApp/app/src/main/java/com/arcgismaps/toolkit/popupapp/screens/mapscreen/MapViewModel.kt

toolkit/popup/api/popup.api

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

kaushikrw · 2025-09-16T21:18:04Z

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

 */
+@Deprecated(
+    message = "Maintained for binary compatibility. Use the overload that uses the PopupState object.",
+    level = DeprecationLevel.HIDDEN


Any particular reason this is hidden? Imo this would be confusing to anyone who upgrades to v300 and is not able to see this function. Not sure what the user story for Popup is but imo having this as a warning with later removal or a full removal in 300.0 might be a better approach.

I vaguely remember Soren mentioning it(I am not 100%). I have changed it to warning for now. Soren can chime in later.

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

...smaps/toolkit/popup/internal/element/utilityassociationselement/UtilityAssociationDetails.kt

...s/toolkit/popup/internal/element/utilityassociationselement/UtilityAssociationGroupResult.kt

...toolkit/popup/internal/element/utilityassociationselement/UtilityAssociationsElementState.kt

.../arcgismaps/toolkit/popup/internal/element/utilityassociationselement/UtilityAssociations.kt

...kit/popup/src/main/java/com/arcgismaps/toolkit/popup/internal/navigation/NavigationAction.kt

add some suggested optimizations

optimize mediaElementState

kaushikrw

Looking good! Just a few more comments below.

...popup/src/main/java/com/arcgismaps/toolkit/popup/internal/element/media/MediaElementState.kt

...popup/src/main/java/com/arcgismaps/toolkit/popup/internal/element/media/MediaPopupElement.kt

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

kaushikrw · 2025-09-19T18:54:29Z

toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/internal/screens/ContentAwareTopBar.kt

+    onBackPressed: () -> Unit,
+    onClose: () -> Unit,
+    modifier: Modifier = Modifier,
+) {


One thing I noticed while running the app is, the popup shows up in an expandable group with an empty title. So I was wondering why the expandable group since there is no group element in popups.

All the element types are displayed as an expandable cards with title being the title of the element. This is a requirement coming from the design
A stretch goal is to have the individual view elements be "collapsible". When collapsed, a popup element view just displays the title of the element.
https://devtopia.esri.com/runtime/common-toolkit/blob/main/designs/Popup/Popup.md#miscellaneous

Ah I see. Perhaps it is a design discussion then, because the collapsed elements do have a title. I will leave that up to you..

add doc

puneet-pdx · 2025-09-19T22:22:08Z

@kaushikrw I have addressed your comments here and also enabled the close button in the micro app. I am looking into updating UNAssociationsScreen with the latest changes.

…pecified string

update AssociationItem properties to display with the latest on FeatureBranch

Change bottomsheet behavior for onDismiss

kaushikrw

Looks good! I just have one comment below.

kaushikrw · 2025-09-23T18:10:06Z

...s/PopupApp/app/src/main/java/com/arcgismaps/toolkit/popupapp/screens/mapscreen/MainScreen.kt

+    LaunchedEffect(scaffoldState.bottomSheetState.currentValue) {
+        if (scaffoldState.bottomSheetState.currentValue == SheetValue.Hidden) {
+            unselectFeature(viewModel.geoElement, viewModel.layer)
+            viewModel.updatePopupState(null)
+            viewModel.setGeoElement(null)
+        }
+    }


These sequence of steps are already happening in the Popup.onDismiss event so it seems redundant to do the same here.

The Popup.onDismiss does not get invoked if the user manually closes the bottomsheet. If that happens then without this there will be no bottomsheet on the screen and there will be a feature selected on the screen. This makes sure that the behavior is consistent for both the workflows.

Update API file

sorenoid

Wow! that's quite an undertaking. I will test it out tomorrow. For now here is my review of the code.

sorenoid · 2025-09-23T23:22:18Z

...s/PopupApp/app/src/main/java/com/arcgismaps/toolkit/popupapp/screens/mapscreen/MainScreen.kt

+            if (viewModel.popupState != null) {
+                Popup(
+                    viewModel.popupState!!,


could use smart casting here rather than the !! operator.

Suggested change

if (viewModel.popupState != null) {

Popup(

viewModel.popupState!!,

val state = viewModel.popupState

if (state != null) {

Popup(

state,

sorenoid · 2025-09-23T23:47:03Z

toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/internal/navigation/PopupNavHost.kt

+                // guard against null value
+                as? UtilityAssociationsElementState ?: return@composable
+            // Display the association details
+            UtilityAssociationDetails(


It seems like this function should not be called if the result and filter are null, rather than passing in the state and returning early from a composable function. Could the parameters be

val result = utilityAssociationsElementState.state.selectedAssociationResult val filter = state.selectedFilterResult?.filter if (result != null && filter != null) { UtilityAssociationDetails( result, filter, modifier = Modifier.fillMaxSize() ) }

sorenoid · 2025-09-23T23:47:48Z

...smaps/toolkit/popup/internal/element/utilityassociationselement/UtilityAssociationDetails.kt

+    val associationResult = state.selectedAssociationResult ?: return
+    val filter = state.selectedFilterResult?.filter ?: return


see comment on this function at the call site

sorenoid · 2025-09-23T23:51:37Z

...smaps/toolkit/popup/internal/element/utilityassociationselement/UtilityAssociationDetails.kt

+        }
+        associationResult.getFractionAlongEdge()?.let { fraction ->
+            FractionAlongEdgeControl(
+                fraction = associationResult.getFractionAlongEdge()!!.toFloat(),


Suggested change

fraction = associationResult.getFractionAlongEdge()!!.toFloat(),

fraction = fraction.toFloat(),

sorenoid · 2025-09-24T00:02:38Z

...toolkit/popup/internal/element/utilityassociationselement/UtilityAssociationsElementState.kt

+    private var _loading: MutableState<Boolean> = mutableStateOf(true)
+
+    private var _filters: MutableState<List<UtilityAssociationsFilterResult>> =
+        mutableStateOf(emptyList())


nitpick: normally a backing var is directly above the val that uses it.

sorenoid · 2025-09-24T00:24:08Z

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

+                    // TODO remove for release
+                    println("encountered element of type ${element::class.java}")


don't forget to remove this!

sorenoid

Great work!

puneet-pdx · 2025-09-24T23:28:26Z

Thanks @kaushikrw and @sorenoid for the reviews! Merging this.

puneet-pdx added 10 commits August 29, 2025 17:48

WIP

dbf2140

WIP

1dac21a

Update Popup and UtilityAssociationsElementState

7327a00

WIP

9b7246d

WIP

f78a605

clean up code

3564098

clean up imports and unused code

14b3590

WIP

09a55c7

remove duplicate title from old popup

bf29a40

remove support for hasEdits from contentAwareTopbar

move popupelementstates creation to PopupState

bc0bed4

replace form references with popup remove caching attachments in popupstate

puneet-pdx self-assigned this Sep 11, 2025

puneet-pdx added 10 commits September 11, 2025 14:55

update micro app

ff80241

optimize imports

a74403a

enable isNavigationEnabled

acada3e

add doc

add support for ignore list

a105da3

add support for dynamic entity

a4d2504

update api file

02fccbd

deprecate popup composable

update micro app

29241d5

update micro app to not show close icon

4cbab74

add copyright

5f1e78b

disable lint for missing translations

Merge branch 'v.next' into puneet/6396_UNAssociationsPopup

a748476

# Conflicts: # toolkit/popup/api/popup.api

puneet-pdx marked this pull request as ready for review September 15, 2025 17:25

puneet-pdx requested a review from kaushikrw September 15, 2025 18:04

puneet-pdx added 2 commits September 15, 2025 17:31

remove parcelize where it is not being used

bb55c82

update API file

Merge branch 'v.next' into puneet/6396_UNAssociationsPopup

a8325e6

kaushikrw requested changes Sep 16, 2025

View reviewed changes

puneet-pdx added 4 commits September 17, 2025 14:29

address code review comments

7209c50

remove support for isNavigationEnabled and NavigationAction

a453234

remove showConfirmationDialog in UtilityAssocationsDetails

1d5cdb7

add support for display count

b54f7d0

add some suggested optimizations

Optimize imports

857f62c

optimize mediaElementState

puneet-pdx requested a review from kaushikrw September 18, 2025 02:01

update logic in mediaPopupElement composable

10ecb16

kaushikrw reviewed Sep 19, 2025

View reviewed changes

enable close button

35a051a

add doc

puneet-pdx requested a review from kaushikrw September 19, 2025 22:22

puneet-pdx added 5 commits September 22, 2025 10:31

add search box to filter UtilityAssociationGroupResults with a user s…

d5e75ae

…pecified string

update showall behavior on search

74c3047

update AssociationItem properties to display with the latest on FeatureBranch

Add fix for navigation not working

76ec033

Change bottomsheet behavior for onDismiss

Add default value for UtilityElement title

c062f86

reset geoElement when the bottomsheet is dismissed

b1dc7e7

kaushikrw approved these changes Sep 23, 2025

View reviewed changes

puneet-pdx added 2 commits September 23, 2025 15:46

Pick up refactor changes from feature forms

c478337

Update API file

update micro app

c582253

puneet-pdx requested a review from sorenoid September 23, 2025 22:51

sorenoid requested changes Sep 24, 2025

View reviewed changes

address code review comments

b328272

puneet-pdx requested a review from sorenoid September 24, 2025 15:55

FIx navigation issue

e8172f6

sorenoid approved these changes Sep 24, 2025

View reviewed changes

puneet-pdx merged commit 6a47e29 into v.next Sep 24, 2025

puneet-pdx deleted the puneet/6396_UNAssociationsPopup branch September 24, 2025 23:28

		val associationResult = state.selectedAssociationResult ?: return
		val filter = state.selectedFilterResult?.filter ?: return

	fraction = associationResult.getFractionAlongEdge()!!.toFloat(),
	fraction = fraction.toFloat(),

		// TODO remove for release
		println("encountered element of type ${element::class.java}")

Add support for UNAssociationsPopupElement to Popup #981

Add support for UNAssociationsPopupElement to Popup #981

Uh oh!

Conversation

puneet-pdx commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Summary of changes:

Pre-merge Checklist

Uh oh!

kaushikrw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kaushikrw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

puneet-pdx commented Sep 19, 2025

Uh oh!

kaushikrw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sorenoid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sorenoid left a comment

Choose a reason for hiding this comment

Uh oh!

puneet-pdx commented Sep 24, 2025

Uh oh!

Uh oh!

puneet-pdx commented Sep 10, 2025 •

edited

Loading