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

Feature: Album duration #666

Merged
merged 4 commits into from
Nov 23, 2024
Merged

Feature: Album duration #666

merged 4 commits into from
Nov 23, 2024

Conversation

sevonj
Copy link
Contributor

@sevonj sevonj commented Nov 15, 2024

Closes #588

Changes

  • Albums now track total duration
  • The duration is shown in album view
screenshot

image

Comments

Possible further improvements:

  • I think the UI would look nicer if the menu moved to the top right corner.

    mockup

    image

  • Custom formatting; don't show seconds if duration >= 1 min

Alternatives:
Display the duration in the top bar along with song count.

@sevonj sevonj marked this pull request as ready for review November 15, 2024 13:57
@Currrupted
Copy link

Currrupted commented Nov 18, 2024

I think both variants look neat, though I'd expect the menu to have that design/position everywhere. I wouldn't put the album duration next to the track count, however. Makes it look messy.

.copy(fontWeight = FontWeight.Bold)
)
}
Spacer(Modifier.weight(1f))
Copy link
Owner

Choose a reason for hiding this comment

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

This will not provide padding when the artists name is long. Add .padding(left = 4.dp) (change the padding according to the necessity).

@sevonj
Copy link
Contributor Author

sevonj commented Nov 23, 2024

I tested the ui with all kinds of strings, and got more consistent results with a two-column layout instead of fiddling with individual row items.

Screenshot From 2024-11-23 19-33-20

@zyrouge
Copy link
Owner

zyrouge commented Nov 23, 2024

I think the design would be much better if we moved the duration to the left. The text near the more options icons looks uncomfortable since they are not aligned.

Text(album.name)
Row(modifier = Modifier.fillMaxWidth()) {
Row {
Column(
Copy link
Owner

Choose a reason for hiding this comment

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

Revert this and add duration as a row.

@sevonj
Copy link
Contributor Author

sevonj commented Nov 23, 2024

I think the design would be much better if we moved the duration to the left. The text near the more options icons looks uncomfortable since they are not aligned.

Screenshot From 2024-11-23 20-03-31

Like a row?

@sevonj
Copy link
Contributor Author

sevonj commented Nov 23, 2024

Maybe the duration could be slightly less bright colored to make it less awkward.

@zyrouge
Copy link
Owner

zyrouge commented Nov 23, 2024

I think the design would be much better if we moved the duration to the left. The text near the more options icons looks uncomfortable since they are not aligned.

Screenshot From 2024-11-23 20-03-31

Like a row?

Yup, that looks for now. We could make changes to the UI in the future.

@zyrouge
Copy link
Owner

zyrouge commented Nov 23, 2024

Maybe the duration could be slightly less bright colored to make it less awkward.

I was thinking the same but we might add more metadata such as album start/end year which would make it look less awkward.

@sevonj
Copy link
Contributor Author

sevonj commented Nov 23, 2024

With the current design, I'd add the metadata to the same row as in

2024-2025 | 2h 49m

@zyrouge
Copy link
Owner

zyrouge commented Nov 23, 2024

With the current design, I'd add the metadata to the same row as in

2024-2025 | 2h 49m

Yeah, that is how I meant. Don't add them in this PR yet.

@sevonj
Copy link
Contributor Author

sevonj commented Nov 23, 2024

This should be OK to merge now.

@zyrouge
Copy link
Owner

zyrouge commented Nov 23, 2024

LGTM!

@zyrouge zyrouge closed this Nov 23, 2024
@zyrouge zyrouge reopened this Nov 23, 2024
@zyrouge zyrouge merged commit f932602 into zyrouge:main Nov 23, 2024
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.

[Feature Request] Total duration of an album
3 participants