[WIP] Move the application shell to shared Compose navigation by sargunv · Pull Request #1 · sargunv/StreetComplete

sargunv · 2026-09-18T19:13:22Z

Pre-review mirror of streetcomplete#7125, based on codex/maplibre-compose-completion so the diff excludes the MapLibre Compose migration. Not intended to merge here.

westnordost · 2026-09-18T20:16:33Z

Hm weird, I merged master into maplibre-compose into maplibre-compose-completion into this branch (😅) but the PR diff is still showing me loads of changes on unrelated metadata (translations, icons, ...)

sargunv · 2026-09-18T20:39:06Z

All the merge commits confused me so I just recreated this branch with a fresh commit on top of the latest streetcomplete#7088

westnordost

(not a complete review as this is still draft)

Have a look at all places where AnimatedScreenVisibility is used. Most of these are in MainScreen. Now that it is completely in compose, it makes sense to have these screens (IntroTutorialScreen, OverlaysTutorialScreen, TeamModeWizard) as own screens in the nav graph.
Not sure what to think of the appFormattingLocale stuff. It seems awfully error prone to sprinkle a check for that global variable everywhere we are doing anything Locale-aware (potentially) outside of the composition. I tend towards that it would be easier if the application class would listen for changes on the (app) locale settings and set the Locale.setDefault(…) on Android/JVM and something similar on iOS.

westnordost · 2026-09-18T21:15:16Z

-            proguardFiles(getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro")
-            testProguardFile("test-proguard-rules.pro")
+            proguardFiles(getDefaultProguardFile("proguard-android-optimize.txt"))
        }
        getByName("debug") {
            isMinifyEnabled = false
-            proguardFiles(getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro")
            applicationIdSuffix = ".debug"


Removing the proguard-rules.pro config seems a bit dangerous to me. I don't know how one could test what of these rules is still needed and which not. As far as I know, if too much is stripped, it doesn't lead to the app not compiling, but to runtime "class/method not found" exceptions.

fixed in 3597865, this snuck in with the rebase

westnordost · 2026-09-18T22:04:10Z

+        enterTransition = { slideInHorizontally(initialOffsetX = { +it * dir }) },
+        exitTransition = { slideOutHorizontally(targetOffsetX = { -it * dir }) },
+        popEnterTransition = { slideInHorizontally(initialOffsetX = { -it * dir }) },
+        popExitTransition = { slideOutHorizontally(targetOffsetX = { +it * dir }) },


You mentioned that this animation feels unnatural. I actually removed this recently to see what would be the default transition but for some reason, it resulted in no animation at all. (Even though there is a default, which doesn't look like "no animation".) Anyway, if you want to play around with this, this is the place.

I spent some time playing with the transitions, and couldn't get it to feel right on Navigation 2.

Will try a Navigation 3 upgrade on another branch.

Alright, we are one the same page/boat then

sargunv · 2026-09-18T23:49:28Z

Not sure what to think of the appFormattingLocale stuff. It seems awfully error prone to sprinkle a check for that global variable everywhere we are doing anything Locale-aware (potentially) outside of the composition. I tend towards that it would be easier if the application class would listen for changes on the (app) locale settings and set the Locale.setDefault(…) on Android/JVM and something similar on iOS.

consolidated in c7f9414

sargunv · 2026-09-19T00:09:25Z

Have a look at all places where AnimatedScreenVisibility is used. Most of these are in MainScreen. Now that it is completely in compose, it makes sense to have these screens (IntroTutorialScreen, OverlaysTutorialScreen, TeamModeWizard) as own screens in the nav graph.

Done in b10a374.

these full screen dialogs have a different transition than the other screens, so I preserved it for now with a conditional in the NavHost transition args.

looking at refining transitions separately

westnordost · 2026-09-19T09:05:14Z

these full screen dialogs have a different transition than the other screens, so I preserved it for now with a conditional in the NavHost transition args.

Right, this came to my mind this night, too. In the end, it remains to be seen whether it makes more sense as a full screen "dialog" or as part of the nav graph. On the other hand, it is part of the navigation (i.e. back stack etc) so it is probably more idiomatic to have it there.

westnordost · 2026-09-22T15:42:19Z

Sign in to view

However, the method seems to be widely used, so, I think it's fine to use it. If it causes app store rejection, it could still be changed. (I.e. the language selection feature disabled for iOS)

More research on this:

On both Android 13 onwards and iOS 17 onwards, it is possible to set the app language from the system settings. To enable:

Android: in the manifest, add android:localeConfig="@xml/locale_config", where locale_config is a file that needs to include a list of supported languages as documented here.

iOS: setting UIPrefersShowingLanguageSettings in the Info.plist to yes. iOS doesn't provide an API to set/synchronize this with an in-app language selector.

For Android, I have actually looked into supporting it two years ago
streetcomplete#5623 (comment) and decided against it.

Anyway, the research doesn't change that I think the current path (in-app language selector) is the best path forward. Only, if the app is rejected because of this, we now know an alternative (for iOS).

I noticed AppleLanguages was being set recursively, causing a crash. a30a303 resolves

For Android, I have actually looked into supporting it two years ago

that's some interesting research; it's disappointing even the android api for this is not well suited

…d sidewalks (streetcomplete#7139) * Fix: Sidewalk Surface tagging bug * Undo previous changes * Rectify one-sided sidewalk surface tagging * Add tests for sidewalk surface tag rectification * Implement requested changes * Update tests to faciliate changes * Add isResurvey indicator

…gestion (fixes streetcomplete#7135)

sargunv mentioned this pull request Sep 18, 2026

[WIP] Move the application shell to shared Compose navigation streetcomplete/StreetComplete#7125

Draft

Move the application shell to shared Compose navigation

bb184fc

sargunv force-pushed the sargunv/compose-multiplatform branch from 3970c77 to bb184fc Compare September 18, 2026 20:37

sargunv added 2 commits September 18, 2026 13:40

Remove the remaining-work notes

c5264ef

Tidy the shell's navigation and environment wiring

0e3fb70

westnordost reviewed Sep 18, 2026

View reviewed changes

sargunv added 2 commits September 18, 2026 16:41

Keep the app's ProGuard rules

3597865

Apply the app language from the application, not the composition

c7f9414

Show the tutorials and team mode wizard as navigation destinations

b10a374

This comment was marked as resolved.

Sign in to view

westnordost reviewed Sep 20, 2026

View reviewed changes

Amaanprobably and others added 9 commits September 22, 2026 23:41

make quest form peek relative to screen height as per @mcliquid's sug…

93c29ad

…gestion (fixes streetcomplete#7135)

apply suggestion by @andrewharvey in streetcomplete#7133

d925b57

Shorten hint text and limit it to one line (fixes streetcomplete#7132)

f9a7c04

lighter button colors in dark mode (fixes streetcomplete#7130)

6c498f6

oops, fix language selector didn't work

0193063

Merge MapLibre migration updates into the shared Compose shell

0b8fe44

Merge current master and its locale selection fix

0d55a15

Keep iOS app language overrides process-local

a30a303

Conversation

sargunv commented Sep 18, 2026

Uh oh!

westnordost commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sargunv commented Sep 18, 2026

Uh oh!

westnordost left a comment

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sargunv commented Sep 18, 2026

Uh oh!

sargunv commented Sep 19, 2026

Uh oh!

westnordost commented Sep 19, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westnordost Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

More research on this:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

westnordost commented Sep 18, 2026 •

edited

Loading

westnordost Sep 22, 2026 •

edited

Loading