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

Implement watch downloads screen #854

Merged
merged 4 commits into from
Mar 31, 2023

Conversation

mchowning
Copy link
Contributor

Description

This fills in the Downloads screen on the watch app.

In addition, this fixes an issue where after deleting a download, we would navigate back too far instead of just going back to the episode screen that initiated the deletion.

Testing Instructions

In addition to the following testing steps, the UI should also be in good shape as well.

  1. Without any downloads, go to the downloads screen in the watch app.
  2. Observe that the screen presents a message indicating you have no downloads.
  3. Log into an account and download some episodes (via the download button on the screen for a specific episode, can be accessed via Podcasts → Select a podcast → Select an episode → tap on the Download "down arrow" icon).
  4. Return to the download screen and observe that the downloaded episodes are listed.
  5. Tap on one of the downloaded episodes
  6. Observe that you are taken to the episode screen for that episode
  7. Delete the download from the episode screen by tapping the download checkmark to the left of the play button.
  8. Swipe back to the download screen
  9. Observe that the episode with the just-deleted download is no longer included in the download screen.

Screenshots or Screencast

Screen.Recording.2023-03-30.at.11.53.42.AM.mov

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 different themes
  • with a landscape orientation
  • with the device set to have a large display and font size
  • for accessibility with TalkBack

@mchowning mchowning added the [Area] Wear OS watch app label Mar 30, 2023
@mchowning mchowning requested a review from a team as a code owner March 30, 2023 15:54
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.

Looks and works great! I just left minor comments.

Feel free to merge it even if you decide not to make any changes.

) {

val viewModel = hiltViewModel<DownloadsScreenViewModel>()
val state by viewModel.stateFlow.collectAsState()
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that collectAsStateWithLifecycle() is available with the recent lifecycle version bump, wdyt about replacing collectAsState() with it?

),
maxLines = 2,
)
val shortDate = episode.publishedDate.toLocalizedFormatPattern("dd MMM")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: I see that if the duration is less than an hour, mins are displayed slightly differently:

Screenshot 2023-03-31 at 9 08 26 AM

No need to fix it right now, just mentioning it as I noticed it in Figma.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I'll confirm with Adam.

@mchowning mchowning merged commit de515f7 into add/wear Mar 31, 2023
@mchowning mchowning deleted the add/implement-watch-downloads-screen branch March 31, 2023 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants