Skip to content

Conversation

@tituscmd
Copy link
Contributor

@tituscmd tituscmd commented May 11, 2025

Heyo!
This PR is a small redesign of the Music app.

I changed the title to be above the artist, as done in most if not all music apps. I also made the artist text a bit darker than the title to differentiate them a little more.
I also changed the buttons to be gray (bgAlt). I think judging from the rest of the UI, the aqua buttons are outdated. I'm more than open to debate on that, tho—please share your opinion! :)

But there's one thing I don't know how to fix. As seen in the first image, the "Waiting for track information.." or rather "track information.. Waiting for" is also messed up. I couldn't find any reference of that text in the code, maybe someone can tell me where to switch those as well.

image
music_app_redesign_2

(First iteration)
music_app_redesign_1

@github-actions
Copy link

github-actions bot commented May 11, 2025

Build size and comparison to main:

Section Size Difference
text 378996B -116B
data 944B 0B
bss 22536B 0B

Run in InfiniEmu

@tituscmd tituscmd changed the title Music App Redesign (small) Music App Redesign May 11, 2025
@tituscmd
Copy link
Contributor Author

Here's another pic of it on-device
image

Copy link
Contributor

@minacode minacode left a comment

Choose a reason for hiding this comment

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

Not tested on device but looks good and I like it. Thank you for the contribution :)

@tituscmd
Copy link
Contributor Author

tituscmd commented May 12, 2025

Not tested on device but looks good and I like it. Thank you for the contribution :)

Thank you! I'm glad you like it :)
Do you have any idea how to fix the template text lines being flipped?

@minacode
Copy link
Contributor

It's defined in src/components/ble/MusicService.h in lines 74-76.

@liamcharger
Copy link
Contributor

Looks good!

Wouldn't mind seeing the template strings changed to just say "Not Playing" (by setting the bottom string).

At the bare minimum "This is a very long getTrack name" should be replaced, as you can see it flash when the app is opened.

…oach to switching between skipping and volume buttons
@tituscmd
Copy link
Contributor Author

tituscmd commented May 12, 2025

Wouldn't mind seeing the template strings changed to just say "Not Playing" (by setting the bottom string).

00f51e01-bf0d-45bb-8281-4051fe13582c
This is the updated default screen now.

@tituscmd
Copy link
Contributor Author

tituscmd commented May 12, 2025

I've changed the way to toggle between the track and volume buttons from swiping to double tapping the screen. It also gives a little vibration when the buttons have switched.

I like this more than the swiping. What do y'all think?

@mark9064 mark9064 added the enhancement Enhancement to an existing app/feature label May 12, 2025
@minacode
Copy link
Contributor

I would rather stick to the same patterns as everywhere else to make the controls as intuitive as possible. Everything you have to discover and remember about the UI is something that people will miss or forget.
Why don't we use swipe with the page indicator on the right?

@tituscmd
Copy link
Contributor Author

I feel like it doesn't make sense that swiping an entire page uses the same "mechanic" as toggling through 2 button states.

@minacode
Copy link
Contributor

From my perspective we can break this down to two questions:

  1. How can we make people realize that there are more, but currently hidden, features?
  2. What would we expect people to do to get there?

I would solve the first a visual hint. Because otherwise you would have to read the manual and remember it.

My answer to the second question is that people try the thing that is most intuitive first. Which is in our case, what they are already doing all the time: swiping. I don't think that double tapping is intuitive at all and would suspect any non-techie to never find out about that feature.

@minacode
Copy link
Contributor

But for the scope of this PR it's probably best if you only swap the two labels and leave out the new toggle. I am open to discuss this on another PR. In the meantime we can hopefully merge this one sooner.

@SteveAmor
Copy link
Contributor

I would rather stick to the same patterns as everywhere else to make the controls as intuitive as possible. Everything you have to discover and remember about the UI is something that people will miss or forget. Why don't we use swipe with the page indicator on the right?

#1875 adds a page indicator

@tituscmd
Copy link
Contributor Author

tituscmd commented May 14, 2025

But for the scope of this PR it's probably best if you only swap the two labels and leave out the new toggle. I am open to discuss this on another PR. In the meantime we can hopefully merge this one sooner.

I've done this and reverted back to the original way of switching the labels. I think this PR should be all good now

@mark9064
Copy link
Member

mark9064 commented Jun 2, 2025

Had a quick look with InfiniEmu, the buttons seem darker compared to other bgAlt buttons. Could the shade of grey be made the same?

@tituscmd
Copy link
Contributor Author

tituscmd commented Jun 6, 2025

Had a quick look with InfiniEmu, the buttons seem darker compared to other bgAlt buttons. Could the shade of grey be made the same?

Yep! Looks like this now, I like it:
image

@mark9064 mark9064 added this to the 1.16.0 milestone Jun 8, 2025
Copy link
Contributor

@minacode minacode left a comment

Choose a reason for hiding this comment

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

LGTM

@minacode minacode merged commit c3afbc5 into InfiniTimeOrg:main Jun 29, 2025
7 checks passed
@minacode
Copy link
Contributor

Thank you, for your contribution, @tituscmd! :)

@tituscmd
Copy link
Contributor Author

Thank you, for your contribution, @tituscmd! :)

You're welcome! Glad I could contribute :)

tmaklin pushed a commit to tmaklin/InfiniTime that referenced this pull request Jul 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhancement to an existing app/feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants