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

Add watch settings screen with metered data warning #860

Merged
merged 3 commits into from
Apr 3, 2023

Conversation

mchowning
Copy link
Contributor

@mchowning mchowning commented Apr 1, 2023

Description

Add settings screen for watch app and implements a setting to give a warning before streaming an episode.

Testing Instructions

  1. Pair your watch to a phone
  2. Go to the settings screen in the watch
  3. Check the UI for the log in button
  4. Log into the app
  5. Check the UI for the log out button
  6. Toggle the metered data warning on
  7. Check the UI for the metered data warning on state
  8. With the connected phone using a metered connection, try to play an episode on the watch (if playing from the now playing screen, the currently queued episode should play, if from the episode details screen, that episode should get loaded and played).
  9. Confirm that you get a metered data warning before playback starts (you will only get this warning the first time you play a particular episode)
  10. Switch the phone to an unmetered connection
  11. Try to play a different episode and confirm you do not get a metered data warning.
  12. From the settings, toggle the metered data warning off
  13. Try to play a different episode
  14. Confirm that you do not get a metered data warning even if the phone is using metered data

Screenshots or Screencast

Logged in, setting off Logged out, setting on
Screenshot 2023-04-01 at 11 14 49 AM Screenshot 2023-04-01 at 11 12 52 AM

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • I have considered whether it makes sense to add tests for my changes
  • All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
  • Any jetpack compose components I added or changed are covered by compose previews

I have tested any UI changes...

  • with the device set to have a large display and font size
  • for accessibility with TalkBack

@mchowning mchowning requested a review from a team as a code owner April 1, 2023 15:19
Comment on lines +72 to +83
// Listen for results from streaming confirmation screen
navController.currentBackStackEntry?.savedStateHandle
?.getStateFlow<StreamingConfirmationScreen.Result?>(StreamingConfirmationScreen.resultKey, null)
?.collectAsStateWithLifecycle()?.value?.let { streamingConfirmationResult ->
val viewModel = hiltViewModel<NowPlayingViewModel>()
LaunchedEffect(streamingConfirmationResult) {
viewModel.onStreamingConfirmationResult(streamingConfirmationResult)
// Clear result once consumed
navController.currentBackStackEntry?.savedStateHandle
?.remove<StreamingConfirmationScreen.Result?>(StreamingConfirmationScreen.resultKey)
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't love the boilerplate this requires to get a result from the stream warning screen via the savedStateHandle. This seemed like the best solution though since I can't use a shared view model due to the fact that I need different behavior depending on whether the stream warning is showing from the now playing screen or the episode details screen (now playing plays the currently queued episode, epsidoe details queues and plays the currently viewed episode).

Comment on lines +98 to +101
navController.previousBackStackEntry?.savedStateHandle?.set(
StreamingConfirmationScreen.resultKey,
result
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is passing the result of what the user selected back to the previous screen.

Copy link
Contributor

@ashiagr ashiagr left a comment

Choose a reason for hiding this comment

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

Nice work!
UI for the log in/ log out button matches Figma specs. Also, streaming on metered network displayed metered data warning:

@ashiagr ashiagr merged commit 4a0389d into add/wear Apr 3, 2023
@ashiagr ashiagr deleted the add/watch-settings-screen branch April 3, 2023 10:21
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