Skip to content

Make slider colors themable - #20308

Merged
hrydgard merged 1 commit into
hrydgard:masterfrom
NABN00B:themable-slider-colors
May 13, 2025
Merged

Make slider colors themable#20308
hrydgard merged 1 commit into
hrydgard:masterfrom
NABN00B:themable-slider-colors

Conversation

@NABN00B

@NABN00B NABN00B commented May 1, 2025

Copy link
Copy Markdown
Contributor

Improves #18802 .

My proposed solution for the slider highlight problems.

This is an improved version of what I showed at #18802 (comment)

This implementation allows for, better customization and somewhat accurate simulation of v1.14.4 (or even the current) behavour.

What this PR does:

  • combine all the colors of the slider's filled bar, knob and label into a single one.
  • inside popups the slider now uses 2 new colors*
  • outside popups the slider now uses the same colors as any other GUI item
  • update the default themes to use the new popup slider color definitions (@hrydgard will want to adjust them though, please check)

What this PR doesn't do:

  • change the slider's unfilled bar in any way
  • update my custom themes to use the new popup slider color definitions (coming in a separate PR)

Note:
I only implemented 1 common color for both focused and down popup sliders because we can't directly switch from either state to the other. The same applies to other GUI elements, so I'm assuming this is intended behavour.

Screenshots of focused sliders inside popups.
image

@hrydgard

hrydgard commented May 1, 2025

Copy link
Copy Markdown
Owner

Good stuff!

I will review this properly in a couple of days, sorry for the delay.

@hrydgard hrydgard added this to the v1.19.0 milestone May 1, 2025
@hrydgard hrydgard added the User Interface PPSSPP's own user interface / UX label May 1, 2025
@NABN00B

NABN00B commented May 4, 2025

Copy link
Copy Markdown
Contributor Author

I'm now contemplating whether the knob and the label should be colored differently from the filled bar if the slider is not focused / down ... 🤔

I also forgot to handle the case when the slider isn't enabled.

@hrydgard

hrydgard commented May 6, 2025

Copy link
Copy Markdown
Owner

Hm, well, the disabled case is not very interesting, it never happens now because we disable the button that cause the popup. I think we can leave it for later.

As for the focused, let me know if you want to change it or not. I'll test this first, but from what I can tell I'm fine with how this is turning out.

@NABN00B

NABN00B commented May 13, 2025

Copy link
Copy Markdown
Contributor Author

As for the focused, let me know if you want to change it or not. I'll test this first, but from what I can tell I'm fine with how this is turning out.

I'm ok with the way it is right now for 1.19, I'm worried about over-complicating things if I make further changes.

Don't forget to adjust the slider colors in Default / Dark.

@hrydgard

Copy link
Copy Markdown
Owner

OK!

I'll try to get this in this afternoon.

@hrydgard

hrydgard commented May 13, 2025

Copy link
Copy Markdown
Owner

Hm, trying with Slate Forest:

image

Not sure what focused color to choose.

I sort of feel that it's the right side of the slider that should change color when focused, the light gray could change to something like the color of the popup buttons.

@NABN00B

NABN00B commented May 13, 2025

Copy link
Copy Markdown
Contributor Author

Hm, trying with Slate Forest:

Not sure what focused color to choose.

As I said in op I'll get the updates to my custom themes in later, because some other changes were also made.

@NABN00B

NABN00B commented May 13, 2025

Copy link
Copy Markdown
Contributor Author

image
image
image
This is how they look right now

@hrydgard

Copy link
Copy Markdown
Owner

Oh ok, those are pretty good.

@hrydgard
hrydgard merged commit a87391a into hrydgard:master May 13, 2025
@hrydgard

Copy link
Copy Markdown
Owner

I'll adjust the colors in Default/Dark in my next PR.

This was referenced May 13, 2025
@NABN00B
NABN00B deleted the themable-slider-colors branch July 1, 2025 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

User Interface PPSSPP's own user interface / UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants