Skip to content

Restyle message boxes and add icon support to them on X11. #13267

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

eafton
Copy link

@eafton eafton commented Jun 24, 2025

This patch adds an icon (warning, icon, information) to message boxes on the X11 backend.

@eafton eafton marked this pull request as draft June 24, 2025 01:55
@slouken slouken added this to the 3.x milestone Jun 24, 2025
@eafton eafton changed the title Add message box icons on X11. Restyle message boxes and add icon support to them on X11. Jul 6, 2025
@eafton eafton marked this pull request as ready for review July 6, 2025 17:55
@eafton
Copy link
Author

eafton commented Jul 6, 2025

I have decided to redo the positioning/styling code, any opinions?
Screenshot at 2025-07-06 20-57-38
Screenshot at 2025-07-06 20-57-49
Screenshot at 2025-07-06 20-57-53

@slouken slouken modified the milestones: 3.x, 3.4.0 Jul 11, 2025
@slouken slouken requested a review from icculus July 11, 2025 19:20
@slouken
Copy link
Collaborator

slouken commented Jul 11, 2025

I have decided to redo the positioning/styling code, any opinions? Screenshot at 2025-07-06 20-57-38 Screenshot at 2025-07-06 20-57-49 Screenshot at 2025-07-06 20-57-53

This doesn't add much visual style. Maybe we should use an icon circle background with colors to indicate severity? This more closely matches other platforms.

@eafton
Copy link
Author

eafton commented Jul 11, 2025

I have decided to redo the positioning/styling code, any opinions? Screenshot at 2025-07-06 20-57-38 Screenshot at 2025-07-06 20-57-49 Screenshot at 2025-07-06 20-57-53

This doesn't add much visual style. Maybe we should use an icon circle background with colors to indicate severity? This more closely matches other platforms.

I am looking into it.

@eafton eafton force-pushed the x11messagebox-icons branch from 704a790 to 2c8c04e Compare July 11, 2025 19:39
@eafton
Copy link
Author

eafton commented Jul 11, 2025

I implemented the suggested changes and switched to a circle background, thoughts?
screenshot

@slouken
Copy link
Collaborator

slouken commented Jul 11, 2025

It doesn't look great to me. Maybe it makes sense to build in actual icons?

@eafton
Copy link
Author

eafton commented Jul 11, 2025

It doesn't look great to me. Maybe it makes sense to build in actual icons?

What about using coloring the icons in with blue, red, yellow, etc and drawing an outline?

@eafton
Copy link
Author

eafton commented Jul 11, 2025

It doesn't look great to me. Maybe it makes sense to build in actual icons?

image Is this any better? I think it wouldnt be bad if we used a light color scheme for the widgets.

@eafton
Copy link
Author

eafton commented Jul 11, 2025

It doesn't look great to me. Maybe it makes sense to build in actual icons?

image

What about this?

@slouken
Copy link
Collaborator

slouken commented Jul 11, 2025

It doesn't look great to me. Maybe it makes sense to build in actual icons?

image What about this?

That's not bad. Can you check other platforms to see what their icons look like? It might be worth checking zenity, as that's closer to what people would expect on Linux.

@eafton
Copy link
Author

eafton commented Jul 11, 2025

That's not bad. Can you check other platforms to see what their icons look like? It might be worth checking zenity, as that's closer to what people would expect on Linux.

After checking, I decided to add a small shadow to the icon for increasing contrast, opinions?
image
image
image

@eafton
Copy link
Author

eafton commented Jul 13, 2025

@slouken I decided to tweak the palette some more and bevelling to the buttons, thoughts?
a
a2
a3

@slouken
Copy link
Collaborator

slouken commented Jul 14, 2025

This seems fine. @icculus, thoughts?

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