Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Android): add basic onboarding UX #592

Merged
merged 10 commits into from
Dec 17, 2024
Merged

Conversation

boringcactus
Copy link
Member

@boringcactus boringcactus commented Dec 16, 2024

Summary

Ticket: 🤖 | Onboarding Flow UX

Adds the basic onboarding UX to Android.

iOS

  • If you added any user-facing strings on iOS, are they included in Localizable.xcstrings?
    • Add temporary machine translations, marked "Needs Review"
      android
  • All user-facing strings added to strings resource

Testing

Verified that the fixed onboarding screens appear on first launch and the hide maps onboarding screen appears if TalkBack is turned on. Added unit tests for each individual screen and the overall flow, similar to what exists on iOS.

@boringcactus boringcactus requested a review from a team as a code owner December 16, 2024 23:12
Copy link
Collaborator

@KaylaBrady KaylaBrady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

settingsRepository: ISettingsRepository = koinInject()
) {
val coroutineScope = rememberCoroutineScope()
var sharingLocation by remember { mutableStateOf(false) }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Is it safe to use remember instead of rememberSaveable here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not! If the orientation changes, the permission response is ignored, because this will have been forgotten. Good catch.

class OnboardingScreenViewTest {
@get:Rule val composeTestRule = createComposeRule()

// We can't easily mock the permission request, so we grant the permission eagerly.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 nice

map.mapboxMap.addOnMapClickListener { point -> handleStopClick(map, point) }
map.location.setLocationProvider(locationProvider)
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After completing the onboarding flow, the LocationDataManager.currentLocation already has the current location when the HomeMapView is being initialized, so if we don't setLocationProvider until after we've already collected, the first location will be completely lost, the puck won't show, and recentering will not work. (This was not fun to track down.)

@boringcactus boringcactus merged commit f8aaf69 into main Dec 17, 2024
7 checks passed
@boringcactus boringcactus deleted the mth-android-onboarding-ux branch December 17, 2024 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants