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

Allow previewing audio of playlist items #28333

Merged
merged 7 commits into from
May 27, 2024
Merged

Conversation

peppy
Copy link
Member

@peppy peppy commented May 27, 2024

Minimal implementation. Sizing of progress circle isn't great but functionality is there.

osu.2024-05-27.at.08.10.00.mp4

Addresses #21315.

@bdach bdach self-requested a review May 27, 2024 09:41
@bdach
Copy link
Collaborator

bdach commented May 27, 2024

How much should I be nitpicking the visuals? Because to me that looks... not great...

Progress circle aside:

  • comically little padding between the preview thumbnail and the difficulty icon, especially compared to everything else on the playlist item having humongous amounts of padding
  • track title / artist name having even less space to show than previously

@peppy
Copy link
Member Author

peppy commented May 27, 2024

track title / artist name having even less space to show than previously

I reduced the font size to allow for more to display (it was quite oversized before).

comically little padding between the preview thumbnail and the difficulty icon

it's using half the previous padding (looked okay to me). trying to strike a balance between padding and information density. can increase back to 8px if preferred.

@bdach
Copy link
Collaborator

bdach commented May 27, 2024

it's using half the previous padding (looked okay to me). trying to strike a balance between padding and information density. can increase back to 8px if preferred.

We can reduce paddings but at the least if the difficulty icon is going to be that close to the preview, then the text on its right should be at least as close. Or the padding on the left should be slightly more, at the right padding's expense. Currently it just looks completely off imo.

image

@peppy peppy force-pushed the preview-playlist-items branch from 08ac3aa to 75d961e Compare May 27, 2024 11:30
@bdach
Copy link
Collaborator

bdach commented May 27, 2024

I pushed one more visual change to fix this:

before after
1716809061 1716809630

(look at relative alignment of the mapper text and the mod icons). Made apparent by the font size change no doubt.

Seems okay otherwise for a start I suppose

@bdach bdach merged commit c6c66fb into ppy:master May 27, 2024
9 of 17 checks passed
@peppy peppy deleted the preview-playlist-items branch May 30, 2024 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants