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(stats): use bottom sheet #4482

Merged
merged 5 commits into from
Aug 15, 2023

Conversation

FineFindus
Copy link
Contributor

Updated the Stats for Nerds dialog to use a bottom sheet instead of a dialog. This is more consistent with the rest of the application design, which mostly uses bottom sheets instead of dialogs.
Using a bottom sheet also improves the display of the stats in landscape mode.

Before After
Dialog showing stats for nerds Stats for nerds shown in a bottom sheet

@Bnyro
Copy link
Member

Bnyro commented Aug 15, 2023

I think it's fine to move it to a bottom sheet, but

  1. We should keep the title present at the top of the bottom sheet, as in the dialog
  2. I'd prefer keeping the text fields as before

@FineFindus
Copy link
Contributor Author

Stats for Nerds sheet with title and text fields

I'd prefer keeping the text fields as before

I'm not sure about this one. Having disabled text fields without any context to the user can be pretty disorienting. The M3 Design Guidelines suggest marking them as read-only, which would inform the user that the text fields cannot be edited.

@Bnyro
Copy link
Member

Bnyro commented Aug 15, 2023

Imo, the title should not be centered.

The text fields make the dialog feel much better and less empty, I still think that it's the best choice.

@FineFindus
Copy link
Contributor Author

Start-aligned title

I agree that the dialog looks a bit empty otherwise, maybe a larger font/adjusted layout would solve that? I'm not a designer, so I'm not really the right person to solve it, I was just super confused, why the stats were text fields, as they cannot be changed by the user.

@Bnyro
Copy link
Member

Bnyro commented Aug 15, 2023

I think it's just fine like as it is now, I don't think users would be confused by this in general. Other popular Android music players, for example Auxio, do something similar for displaying stats.

@Bnyro Bnyro merged commit 4717098 into libre-tube:master Aug 15, 2023
4 checks passed
@FineFindus FineFindus deleted the feat/stats-for-nerds-sheet branch August 15, 2023 13:16
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