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 PlaylistScreen #352

Merged
merged 1 commit into from
Jul 8, 2022
Merged

Add PlaylistScreen #352

merged 1 commit into from
Jul 8, 2022

Conversation

luizgrp
Copy link
Member

@luizgrp luizgrp commented Jul 8, 2022

WHAT

Add PlaylistScreen.

Screen Shot 2022-07-08 at 13 25 33

WHY

In order to provide common screens.

HOW

  • Add PlaylistUiModel;
  • Change DownloadPlaylistUiModel to contain a PlaylistUiModel;
  • Add PlaylistScreen implementation;
  • Add previews in debug folder;
  • Change BrowseScreen to pass largeIcon = true to SecondaryChip;
  • Change SecondaryChip param name to label in order to align with param name from material Chip;

Checklist 📋

  • Add explicit visibility modifier and explicit return types for public declarations
  • Run spotless check
  • Run tests
  • Update metalava's signature text files

@luizgrp luizgrp requested a review from yschimke July 8, 2022 13:03
@luizgrp luizgrp force-pushed the mediaui_playlist_screen branch from 52222de to 13c1f68 Compare July 8, 2022 13:40
Copy link
Collaborator

@yschimke yschimke left a comment

Choose a reason for hiding this comment

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

Shouldn't this be playlists (plural) in the code?

This also gets into naming, might be worth checking with Danny. Maybe its Collection or some other term.

@luizgrp
Copy link
Member Author

luizgrp commented Jul 8, 2022

Shouldn't this be playlists (plural) in the code?

asked in Figma

This also gets into naming, might be worth checking with Danny. Maybe its Collection or some other term.

this is a broader strategy to be defined for the whole lib and not just for this PR - needs to be defined and iterated over all the default texts

@luizgrp luizgrp force-pushed the mediaui_playlist_screen branch from 13c1f68 to 8b02bae Compare July 8, 2022 14:49
@luizgrp luizgrp merged commit 53f5026 into google:main Jul 8, 2022
@luizgrp luizgrp deleted the mediaui_playlist_screen branch July 8, 2022 15:40
@luizgrp luizgrp self-assigned this Dec 8, 2022
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