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

Match ImGui inherit_base_colors #78740

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

alef
Copy link
Contributor

@alef alef commented Dec 23, 2024

Summary

Interface "Match ImGui inherit_base_colors"

Purpose of change

I started using imgui_style.json:inherit_base_colors = true, but the UI colors didn't match so I changed the three you see in the commit. Here is the main menu loading a world.
Before ... :
Screenshot 2024-12-23 154033
... and after :
Screenshot 2024-12-23 155800

Describe the solution

Describe alternatives you've considered

Testing

Settings "inherit_base_colors" to true in the file imgui_style.json.

Additional context

@github-actions github-actions bot added Info / User Interface Game - player communication, menus, etc. [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Dec 23, 2024
@db48x
Copy link
Contributor

db48x commented Dec 27, 2024

Although I applaud the initiative, I request that you not make this specific change. The titlebar of a window should not be black; it should be some color that contrasts with the window background (which is black).

If you want to match the look of the old window in this case, just modify the code in main_menu.cpp so that it doesn’t give the window a title at all. Like this:

@@ -1254,7 +1254,7 @@ bool main_menu::load_character_tab( const std::string &worldname )
     }
 
     uilist mmenu;
-    mmenu.title = string_format( _( "Load character from \"%s\"" ), worldname );
+    mmenu.text = string_format( _( "Load character from \"%s\"" ), worldname );
     mmenu.border_color = c_white;
     int opt_val = 0;
     for( const save_t &s : savegames ) {

@Kilvoctu
Copy link
Contributor

The titlebar of a window should not be black; it should be some color that contrasts with the window background (which is black).

Are there any places in which this would cause problem?
I tried this change on my fork and checked a number of affected pop-ups (missions, notes, item info, etc.). In each case, I felt overall it looked better; more consistent with the rest of the UI and historical CDDA aesthetic.

@Brambor
Copy link
Contributor

Brambor commented Dec 27, 2024

Somewhat related issues:

The problem is just choosing pleasant colours.

@db48x
Copy link
Contributor

db48x commented Dec 28, 2024

Are there any places in which this would cause problem?

Titlebars are an important user control even though we cannot fully use them at the moment. Generally they allow the user to drag the window to reposition it. We’ve temporarily disabled that because it confuses the code that handles mouse clicks, but eventually I want that to work correctly. Therefore we should not make titlebars invisible. If you think it is important that this window look exactly like the old window, then remove the titlebar instead of making the titlebar invisible.

@GuardianDll
Copy link
Member

Can you rebase it please?

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants