Migrate Offline Area Screens to Compose - #3915
Open
shobhitagarwal1612 wants to merge 3 commits into
Open
Conversation
- Replace offline_area_selector_frag.xml with Compose OfflineAreaSelectorScreen - Convert LiveData to StateFlow with OfflineAreaSelectorState and OfflineAreaSelectorEvent - Inline and modernize DownloadProgressDialog inside OfflineAreaSelectorScreen - Remove legacy viewport outline drawable - Add comprehensive Compose UI and ViewModel unit tests
- Replace offline_area_viewer_frag.xml with Compose OfflineAreaViewerScreen - Convert LiveData to StateFlow with OfflineAreaViewerState - Modernize deletion flow and progress overlay in Compose - Add comprehensive Compose UI and ViewModel unit tests
- Replace offline_areas_frag.xml with Compose OfflineAreasScreen - Convert LiveData to StateFlow with OfflineAreasState - Inline and modernize OfflineAreaListItem inside OfflineAreasScreen - Clean up obsolete layouts, styles, and dimensions - Add comprehensive Compose UI and ViewModel unit tests
shobhitagarwal1612
requested a deployment
to
e2e-approval
September 1, 2026 09:14 — with 
GitHub Actions
Waiting
GitHub Actions
Waiting
| Box(modifier = modifier.fillMaxSize()) { | ||
| Column(modifier = Modifier.fillMaxSize()) { | ||
| Toolbar( | ||
| stringRes = R.string.offline_area_selector_title, |
Collaborator
There was a problem hiding this comment.
nit: the title in the toolbar was previously centered and now it is left aligned
| } | ||
|
|
||
| @Composable | ||
| fun DownloadProgressDialog( |
Collaborator
There was a problem hiding this comment.
shouldn't this also be annotated with @VisibleForTesting?
| offlineAreaRepository.removeFromDevice(deletedArea) | ||
| _uiState.update { it.copy(isProgressOverlayVisible = false, area = null) } | ||
| navigateUpChannel.send(Unit) | ||
| } catch (e: kotlinx.coroutines.CancellationException) { |
Collaborator
There was a problem hiding this comment.
nit: import could be simplified
Comment on lines
+111
to
+112
| Timber.e(e, "Failed to remove offline area") | ||
| _uiState.update { it.copy(isProgressOverlayVisible = false) } |
Collaborator
There was a problem hiding this comment.
it's a nice improvement to have exception handling in this flow now. There is no user feedback in this error case though, maybe we can add an error toast with a generic string (eg. R.string.unexpected_error)?
| modifier = modifier.fillMaxSize(), | ||
| topBar = { | ||
| Toolbar( | ||
| stringRes = R.string.offline_map_imagery, |
Collaborator
There was a problem hiding this comment.
same comment here regarding centering the toolbar title
| imageVector = ImageVector.vectorResource(id = R.drawable.ic_offline_pin), | ||
| contentDescription = stringResource(id = R.string.offline_area_list_item_icon), | ||
| tint = MaterialTheme.colorScheme.primary, | ||
| modifier = Modifier.size(24.dp), |
Collaborator
There was a problem hiding this comment.
nit: consider extracting some reusable dimensions/paddings in these files to Size.kt
| } | ||
|
|
||
| @Composable | ||
| private fun OfflineAreaSelectorButtons( |
Collaborator
There was a problem hiding this comment.
i believe this component may be missing some inset paddings, in devices with a nav bar (screenshot from an API 24 emulator) the buttons get partially hidden behind it
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Towards #1795
Migrates the offline area screens (selector screen, viewer screen & list offline areas screen) from XML to compose layout.
Before:
Screen_recording_20260901_144819.webm
After:
Screen_recording_20260901_144321.webm
@andreia-ferreira PTAL?