You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Implemented a unified SemanticColors namespace to standardize UI state indicators (modified, saved, danger, etc.) across the application. Replaced hardcoded ImVec4 color values throughout dialogs and panels with these semantic constants to ensure theme consistency.
ImguiMapEditor/Application.cpp — injected theme pointer into preferences
ImguiMapEditor/Presentation/MainWindow.cpp — updated tab coloring using semantic constants
ImguiMapEditor/UI/Core/Theme.h — defined SemanticColors namespace with standard state/text roles
ImguiMapEditor/UI/Dialogs/ClientConfiguration/ClientConfigurationDialog.cpp — replaced hardcoded colors with SemanticColors constants
ImguiMapEditor/UI/Dialogs/ClientConfiguration/ClientPropertyEditor.cpp — updated color constants to semantic mapping
ImguiMapEditor/UI/Dialogs/ConfirmationDialog.cpp — included theme header [minor]
ImguiMapEditor/UI/Dialogs/EditTownsDialog.cpp — replaced local color definitions with SemanticColors
ImguiMapEditor/UI/Dialogs/MapCompatibilityPopup.cpp — used semantic constants for warnings
ImguiMapEditor/UI/Dialogs/NewMapDialog.cpp — updated label/text coloring and button styles
ImguiMapEditor/UI/Dialogs/Properties/ItemPropertiesDialog.cpp — included theme header [minor]
ImguiMapEditor/UI/Dialogs/Startup/AvailableClientsPanel.cpp — applied semantic colors to panel headers and status
ImguiMapEditor/UI/Dialogs/Startup/ClientInfoPanel.cpp — updated client info status and label styling
ImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp — applied semantic colors to list items and labels
ImguiMapEditor/UI/Dialogs/Startup/SelectedMapPanel.cpp — updated status and label styling
ImguiMapEditor/UI/Dialogs/Startup/StartupDialog.cpp — refactored startup dialog styling to be theme-aware
ImguiMapEditor/UI/Dialogs/UnsavedChangesModal.cpp — included theme header [minor]
ImguiMapEditor/UI/Panels/BrushSizePanel.cpp — used SemanticColors::SAVED for active toggle
ImguiMapEditor/UI/Panels/NewMapPanel.cpp — updated frame/border styling with semantic colors
ImguiMapEditor/UI/PreferencesDialog.cpp — added theme selection UI and semantic color usage
ImguiMapEditor/UI/Ribbon/Utils/RibbonUtils.h — updated separator color to SemanticColors::MUTED
ImguiMapEditor/UI/Widgets/TilesetGridWidget.cpp — included theme header [minor]
ImguiMapEditor/UI/Windows/BrowseTile/BrowseTileWindow.cpp — used semantic constants for property colors
ImguiMapEditor/UI/Windows/BrowseTile/SpawnCreatureRenderer.cpp — included theme header [minor]
ImguiMapEditor/UI/Windows/IngameBoxWindow.cpp — replaced hardcoded colors with semantic indicators
Summary by CodeRabbit
New Features
Theme selection with live preview in Preferences; chosen theme is applied and persisted.
Style
Added a centralized semantic color system and helpers for consistent theming.
Replaced many hard-coded colors across dialogs, panels, widgets and windows with theme tokens, improving hover/active/disabled states, warnings/errors, and pulsing/modified visuals.
Implemented a unified `SemanticColors` namespace to standardize UI state indicators (modified, saved, danger, etc.) across the application. Replaced hardcoded `ImVec4` color values throughout dialogs and panels with these semantic constants to ensure theme consistency.
- `ImguiMapEditor/Application.cpp` — injected theme pointer into preferences
- `ImguiMapEditor/Presentation/MainWindow.cpp` — updated tab coloring using semantic constants
- `ImguiMapEditor/UI/Core/Theme.h` — defined `SemanticColors` namespace with standard state/text roles
- `ImguiMapEditor/UI/Dialogs/ClientConfiguration/ClientConfigurationDialog.cpp` — replaced hardcoded colors with `SemanticColors` constants
- `ImguiMapEditor/UI/Dialogs/ClientConfiguration/ClientPropertyEditor.cpp` — updated color constants to semantic mapping
- `ImguiMapEditor/UI/Dialogs/ConfirmationDialog.cpp` — included theme header [minor]
- `ImguiMapEditor/UI/Dialogs/EditTownsDialog.cpp` — replaced local color definitions with `SemanticColors`
- `ImguiMapEditor/UI/Dialogs/MapCompatibilityPopup.cpp` — used semantic constants for warnings
- `ImguiMapEditor/UI/Dialogs/NewMapDialog.cpp` — updated label/text coloring and button styles
- `ImguiMapEditor/UI/Dialogs/Properties/ItemPropertiesDialog.cpp` — included theme header [minor]
- `ImguiMapEditor/UI/Dialogs/Startup/AvailableClientsPanel.cpp` — applied semantic colors to panel headers and status
- `ImguiMapEditor/UI/Dialogs/Startup/ClientInfoPanel.cpp` — updated client info status and label styling
- `ImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp` — applied semantic colors to list items and labels
- `ImguiMapEditor/UI/Dialogs/Startup/SelectedMapPanel.cpp` — updated status and label styling
- `ImguiMapEditor/UI/Dialogs/Startup/StartupDialog.cpp` — refactored startup dialog styling to be theme-aware
- `ImguiMapEditor/UI/Dialogs/UnsavedChangesModal.cpp` — included theme header [minor]
- `ImguiMapEditor/UI/Panels/BrushSizePanel.cpp` — used `SemanticColors::SAVED` for active toggle
- `ImguiMapEditor/UI/Panels/NewMapPanel.cpp` — updated frame/border styling with semantic colors
- `ImguiMapEditor/UI/PreferencesDialog.cpp` — added theme selection UI and semantic color usage
- `ImguiMapEditor/UI/PreferencesDialog.h` — added theme pointer tracking
- `ImguiMapEditor/UI/Ribbon/Utils/RibbonUtils.h` — updated separator color to `SemanticColors::MUTED`
- `ImguiMapEditor/UI/Widgets/TilesetGridWidget.cpp` — included theme header [minor]
- `ImguiMapEditor/UI/Windows/BrowseTile/BrowseTileWindow.cpp` — used semantic constants for property colors
- `ImguiMapEditor/UI/Windows/BrowseTile/SpawnCreatureRenderer.cpp` — included theme header [minor]
- `ImguiMapEditor/UI/Windows/IngameBoxWindow.cpp` — replaced hardcoded colors with semantic indicators
Docstring coverage is 11.32% which is insufficient. The required threshold is 80.00%.
Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name
Status
Explanation
Description Check
✅ Passed
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
✅ Passed
The title 'refactor(ui): introduce semantic color system' directly and accurately describes the main change: introducing a centralized semantic color system to replace hardcoded UI colors throughout the codebase.
Linked Issues check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
✅ Passed
Check skipped because no linked issues were found for this pull request.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
Comment @coderabbitai help to get the list of available commands and usage tips.
Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a unified semantic color system to the UI layer, replacing hardcoded color definitions with a centralized namespace. This change significantly improves maintainability and ensures visual consistency across the application's various dialogs, panels, and windows, while also laying the groundwork for more robust theme support.
Highlights
Introduction of Semantic Color System: Defined a centralized SemanticColors namespace in UI/Core/Theme.h to standardize UI state indicators such as MODIFIED, SAVED, DANGER, and WARNING.
Refactoring UI Components: Replaced hardcoded ImVec4 color values with semantic constants across multiple dialogs, panels, and windows to ensure visual consistency and maintainability.
Theme Integration: Updated Application.cpp and PreferencesDialog to support theme-aware UI, enabling dynamic color management across the application.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
Feature
Command
Description
Code Review
/gemini review
Performs a code review for the current pull request in its current state.
Pull Request Summary
/gemini summary
Provides a summary of the current pull request in its current state.
Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help
/gemini help
Displays a list of available commands.
Customization
To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.
Introduces a centralized SemanticColors namespace and integrates it across dialogs, panels, and windows to replace hardcoded ImGui colors, while wiring theme selection through Preferences and removing dialog-specific dark styling in favor of the active theme.
File-Level Changes
Change
Details
Files
Introduce SemanticColors namespace to define semantic UI state and text role colors used across all themes.
Added SemanticColors namespace in Theme.h with state colors (MODIFIED, SAVED, DANGER, WARNING, INFO, GOLD) and text roles (LABEL, VALUE, EMPTY, HEADER_TEXT, MUTED), plus PULSE_BASE and DISABLED_ALPHA.
Documented SemanticColors intent as theme-independent semantic meanings.
Updated other headers to include Theme.h where semantic colors are referenced.
Refactor dialogs and panels to use semantic colors instead of hardcoded ImVec4 values for stateful and informational UI elements.
Replaced direct ImVec4 color literals for labels, values, headers, warnings, errors, and info text with SemanticColors constants (e.g., LABEL, VALUE, HEADER_TEXT, WARNING, DANGER, SAVED, INFO, GOLD, EMPTY, MUTED).
Updated button styles in various dialogs (NewMapDialog, EditTownsDialog, MapCompatibilityPopup, StartupDialog, ClientConfigurationDialog) to use semantic state colors (INFO for primary, DANGER for destructive, WARNING for risky actions, SAVED for success).
Adjusted disabled states to use SemanticColors::DISABLED_ALPHA or ImGui default disabled color instead of manual alpha values.
Centralized tab and toggle status coloring (e.g., map editor tabs, BrushSizePanel active toggles, IngameBoxWindow toggles, browse tile property flags) on SemanticColors meanings.
Introduced local aliases namespace SC = SemanticColors; for concise usage in multiple source files.
Expose theme selection in Preferences and wire the selected ThemeType into app settings.
Extended PreferencesDialog to hold a ThemeType* pointer and setter, plus use Theme.h in the header.
In Application::initialize, passed the app settings theme pointer into PreferencesDialog via setThemePtr and ensured settings are saved via the existing apply callback.
Implemented a new Theme section in PreferencesDialog::renderEditorTab with a collapsing header, combo box listing AVAILABLE_THEMES, icons per theme type, preview colors, tooltips, and description text for the selected theme.
Hooked theme selection to ApplyTheme(theme.type) and updated the stored ThemeType via the theme pointer, then invoked on_apply_settings_ callback to persist changes.
Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 2 issues, and left some high level feedback:
In PreferencesDialog::renderEditorTab, std::ranges::find_if is dereferenced unconditionally when theme_ptr_ is set; consider handling the case where the theme type is not found in AVAILABLE_THEMES to avoid undefined behavior if the list and saved value ever get out of sync.
In AvailableClientsPanel and RecentMapsPanel, the kSelectedHeader* and kUnselectedHeader* colors are still hardcoded dark-theme RGBA values; it would be more robust to derive these from the active theme (or add corresponding semantic roles) so they adapt correctly when new themes are added.
Several buttons (e.g., primary actions in StartupDialog, NewMapDialog, and the compatibility popup) use SC::INFO for all of normal/hover/active states, which removes visual hover/press feedback; consider deriving slightly varied hover/active colors from the semantic base color to preserve interaction affordance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- In `PreferencesDialog::renderEditorTab`, `std::ranges::find_if` is dereferenced unconditionally when `theme_ptr_` is set; consider handling the case where the theme type is not found in `AVAILABLE_THEMES` to avoid undefined behavior if the list and saved value ever get out of sync.
- In `AvailableClientsPanel` and `RecentMapsPanel`, the `kSelectedHeader*` and `kUnselectedHeader*` colors are still hardcoded dark-theme RGBA values; it would be more robust to derive these from the active theme (or add corresponding semantic roles) so they adapt correctly when new themes are added.
- Several buttons (e.g., primary actions in `StartupDialog`, `NewMapDialog`, and the compatibility popup) use `SC::INFO` for all of normal/hover/active states, which removes visual hover/press feedback; consider deriving slightly varied hover/active colors from the semantic base color to preserve interaction affordance.
## Individual Comments### Comment 1
<locationpath="ImguiMapEditor/UI/PreferencesDialog.cpp"line_range="180-181" />
<code_context>
++ if (theme_ptr_) {
+ ImGui::SameLine();+ const auto& current_theme = *std::ranges::find_if(+ AVAILABLE_THEMES, [this](const ThemeInfo& ti) { return ti.type == *theme_ptr_; });+ ImGui::TextUnformatted(current_theme.description);+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against std::ranges::find_if returning end() before dereferencing current_theme.
If *theme_ptr_ holds a theme that isn’t in AVAILABLE_THEMES (e.g., after theme list changes or from older/corrupt config), std::ranges::find_if will return end(), and dereferencing it is UB. Please either handle the end() case (e.g., fallback/default description) or assert that a match must exist before dereferencing, so the code remains safe when AVAILABLE_THEMES or saved settings change.
</issue_to_address>
### Comment 2
<locationpath="ImguiMapEditor/UI/Dialogs/Startup/StartupDialog.cpp"line_range="248-251" />
<code_context>
- ImVec4(0.25f, 0.55f, 0.80f, 1.0f));- ImGui::PushStyleColor(ImGuiCol_ButtonActive,
- ImVec4(0.15f, 0.40f, 0.65f, 1.0f));+ // New Map button (primary action)
+ ImGui::PushStyleColor(ImGuiCol_Button, SC::INFO);
+ ImGui::PushStyleColor(ImGuiCol_ButtonHovered, SC::INFO);
+ ImGui::PushStyleColor(ImGuiCol_ButtonActive, SC::INFO);
if (ImGui::Button(ICON_FA_FILE " New map", kUniformButtonSize)) {
pending_result_.action = Action::NewMap;
</code_context>
<issue_to_address>
**suggestion:** Primary buttons use the same color for normal, hovered, and active states, reducing visual feedback.
For primary actions like New Map and Load Map, ImGuiCol_Button, ImGuiCol_ButtonHovered, and ImGuiCol_ButtonActive are all SC::INFO, which removes hover/pressed feedback and can hurt usability/accessibility. Please derive slightly lighter/darker variants from SC::INFO (or introduce INFO_HOVER/INFO_ACTIVE semantic colors) so button states remain visually distinct while staying on-theme.
Suggested implementation:
```cpp// New Map button (primary action)ImGui::PushStyleColor(ImGuiCol_Button, SC::INFO);
ImGui::PushStyleColor(ImGuiCol_ButtonHovered, SC::INFO_HOVER);
ImGui::PushStyleColor(ImGuiCol_ButtonActive, SC::INFO_ACTIVE);
```To fully implement this suggestion you should also:1. Define `SC::INFO_HOVER` and `SC::INFO_ACTIVE` in your shared style/colors module (where `SC::INFO` is currently defined), making them slightly lighter/darker variants of `SC::INFO` for good hover/pressed contrast.2. Ensure any other primary buttons (e.g. "Load Map") also use `SC::INFO`, `SC::INFO_HOVER`, and `SC::INFO_ACTIVE` consistently for normal/hover/active states.</issue_to_address>
Sourcery is free for open source - if you like our reviews please consider sharing them ✨
The reason will be displayed to describe this comment to others. Learn more.
📋 Review Summary
The PR successfully introduces a semantic color system, which is a great step towards a more maintainable and themeable UI. It replaces many hardcoded colors with meaningful constants.
🔍 General Feedback
Correctness: Identified an unsafe dereference in PreferencesDialog.cpp when looking up theme descriptions.
Maintainability: The Theme.h header is becoming quite large due to inline implementations of theme functions. Consider splitting these into a .cpp file.
Portability: Added a note about missing <cmath> include for sinf in MainWindow.cpp.
Cleanliness: Noted an unnecessary inclusion of imgui_internal.h in Theme.h and some minor formatting/comment issues.
Overall, the architectural direction is excellent.
The reason will be displayed to describe this comment to others. Learn more.
🟡 This file uses sinf, but <cmath> is not explicitly included. While it might be included transitively, it's better to include it for portability and clarity.
The reason will be displayed to describe this comment to others. Learn more.
🔴 Unsafe dereference of std::ranges::find_if result. If *theme_ptr_ happens to be an invalid value (not in AVAILABLE_THEMES), this will result in undefined behavior.
The reason will be displayed to describe this comment to others. Learn more.
🟢 Great job standardizing the UI colors with a semantic system! This will significantly improve maintainability and make theme switching much more reliable.
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request centralizes UI styling by introducing a universal SemanticColors namespace in Theme.h and refactoring various dialogs, panels, and windows to use these semantic constants instead of hardcoded ImVec4 values. It also adds theme selection support to the preferences dialog. The review feedback highlights a critical safety issue in PreferencesDialog.cpp where a search iterator is dereferenced without checking for std::end. Additionally, the reviewer notes that using identical semantic colors for normal, hovered, and active button states eliminates interactive visual feedback, suggesting the addition of Lighten and Darken helper utilities to SemanticColors to dynamically adjust brightness. Other minor improvements include removing redundant style pushes in StartupDialog.cpp and preserving alpha transparency for the ribbon separator.
The reason will be displayed to describe this comment to others. Learn more.
Dereferencing the iterator returned by std::ranges::find_if without checking if it equals std::end(AVAILABLE_THEMES) can lead to undefined behavior or a crash if the theme is not found. Safely check the iterator before dereferencing.
if (theme_ptr_) {
ImGui::SameLine();
const auto it=std::ranges::find_if(
AVAILABLE_THEMES, [this](constThemeInfo&ti) { returnti.type==*theme_ptr_; });
if (it!=std::end(AVAILABLE_THEMES)) {
ImGui::TextUnformatted(it->description);
}
}
The reason will be displayed to describe this comment to others. Learn more.
Using the exact same color for Button, ButtonHovered, and ButtonActive removes all visual feedback when the user hovers over or clicks the button. Use the Lighten and Darken helper functions to restore interactive feedback.
The reason will be displayed to describe this comment to others. Learn more.
Defining the normal and hovered button colors as the exact same semantic color removes visual feedback when the user hovers over the buttons. Use the Lighten helper function to restore interactive feedback.
The reason will be displayed to describe this comment to others. Learn more.
Using the exact same color for Button, ButtonHovered, and ButtonActive removes all visual feedback when the user hovers over or clicks the button. Use the Lighten and Darken helper functions to restore interactive feedback.
The reason will be displayed to describe this comment to others. Learn more.
Using the exact same color for Button, ButtonHovered, and ButtonActive removes all visual feedback when the user hovers over or clicks the button. Use the Lighten and Darken helper functions to restore interactive feedback.
The reason will be displayed to describe this comment to others. Learn more.
Using the exact same color for Button, ButtonHovered, and ButtonActive removes all visual feedback when the user hovers over or clicks the button. Use the Lighten and Darken helper functions to restore interactive feedback.
The reason will be displayed to describe this comment to others. Learn more.
Using the exact same color for Button, ButtonHovered, and ButtonActive removes all visual feedback when the user hovers over or clicks the button. Use the Lighten and Darken helper functions to restore interactive feedback.
The reason will be displayed to describe this comment to others. Learn more.
Defining the normal, hovered, and active button colors as the exact same semantic color removes all visual feedback when the user interacts with the buttons. Use the Lighten and Darken helper functions to restore interactive feedback.
The reason will be displayed to describe this comment to others. Learn more.
Pushing the default style colors (ImGui::GetStyleColorVec4) onto the ImGui stack is completely redundant because ImGui will naturally use these colors if no style colors are pushed. Removing these redundant pushes simplifies the code and avoids unnecessary style stack operations.
if (ImGui::Button(ICON_FA_POWER_OFF" Exit", kUniformButtonSize)) {
pending_result_.action=Action::Exit;
}
The reason will be displayed to describe this comment to others. Learn more.
The original separator had an alpha of 0.5f to keep it visually subtle. Changing it to SemanticColors::MUTED directly makes it fully opaque (1.0f alpha), which may look too prominent. Consider preserving the 0.5f alpha while using the semantic color components.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ImguiMapEditor/UI/Dialogs/Startup/AvailableClientsPanel.cpp`:
- Around line 12-16: The four hardcoded ImVec4 constants (kSelectedHeader,
kSelectedHeaderHover, kUnselectedHeader, kUnselectedHeaderHover) should be
replaced with theme-derived values so headers follow the active semantic/theme
colors (also apply the same change where these are repeated around the 46–50
region); locate the constant definitions and replace the literal initializers
with calls to your theme/color API (for example retrieve ImVec4 via
ImGui::GetStyleColorVec4 or your project's Theme::GetColor/semantic lookup using
the appropriate semantic keys like Header/HeaderHovered/HeaderActive) so the
panel adapts to light/dark themes rather than using fixed dark-biased RGBA
literals.
In `@ImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp`:
- Around line 11-15: The constants kSelectedHeader, kSelectedHeaderHover,
kUnselectedHeader and kUnselectedHeaderHover hardcode dark-theme colors; replace
them with colors sampled from the current ImGui style (e.g., derive from
ImGuiCol_Header / ImGuiCol_HeaderHovered / ImGuiCol_HeaderActive via
ImGui::GetStyleColorVec4 or style.Colors[]) so the panel respects
light/dark/custom themes, and compute any selection/hover variants at runtime
(optionally using ImLerp to blend). Update both the declarations
(kSelectedHeader etc.) and the other set used around lines 38–43 to use the
runtime-derived colors instead of fixed constexpr values.
In `@ImguiMapEditor/UI/PreferencesDialog.cpp`:
- Around line 178-183: The code in PreferencesDialog.cpp dereferences the result
of std::ranges::find_if when resolving AVAILABLE_THEMES into current_theme,
which can be undefined if the theme value isn't found; change the logic in the
block that checks theme_ptr_ to first store the iterator/result (auto it =
std::ranges::find_if(AVAILABLE_THEMES, ...)), test it against
std::end(AVAILABLE_THEMES) (or use it != AVAILABLE_THEMES.end()), and only
access it->description when found—otherwise handle the missing-theme case (e.g.,
skip ImGui::TextUnformatted, show a fallback string, or log an error). Ensure
you still respect the existing theme_ptr_ null check and update references to
current_theme accordingly.
In `@ImguiMapEditor/UI/Ribbon/Utils/RibbonUtils.h`:
- Around line 3-4: The header is missing the <format> include used by
std::format in this file; open ImguiMapEditor/UI/Ribbon/Utils/RibbonUtils.h and
add `#include` <format> near the other standard includes so the uses of
std::format (the calls to std::format found in this header) compile regardless
of include order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 148e5c8c-a2a1-438f-8d4b-506454e67aa6
📥 Commits
Reviewing files that changed from the base of the PR and between b0f0c2a and 7d94a34.
…states
Added Lighten/Darken helper functions to SemanticColors for consistent hover/active button states and replaced manual color overrides with dynamic variants across UI dialogs. Removed redundant local color definitions in favor of system-wide constants and added safety checks for theme resolution.
- `ImguiMapEditor/Presentation/MainWindow.cpp` — added `<cmath>` include
- `ImguiMapEditor/UI/Core/Theme.h` — introduced `Lighten` and `Darken` `constexpr` helpers
- `ImguiMapEditor/UI/Dialogs/ClientConfiguration/ClientConfigurationDialog.cpp` — switched to dynamic `Lighten`/`Darken` variants for button accents
- `ImguiMapEditor/UI/Dialogs/ClientConfiguration/ClientPropertyEditor.cpp` — used `Lighten` helper for button hover state
- `ImguiMapEditor/UI/Dialogs/EditTownsDialog.cpp` — updated button hover/active states to use semantic variants
- `ImguiMapEditor/UI/Dialogs/MapCompatibilityPopup.cpp` — updated warning button states to use semantic variants
- `ImguiMapEditor/UI/Dialogs/NewMapDialog.cpp` — updated create map button hover state to use `Lighten` variant
- `ImguiMapEditor/UI/Dialogs/Startup/AvailableClientsPanel.cpp` — switched header colors to use system-defined active/hover styles, removed local constants
- `ImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp` — switched header colors to use system-defined active/hover styles, removed local constants
- `ImguiMapEditor/UI/Dialogs/Startup/StartupDialog.cpp` — simplified button style management and updated sidebar/footer button states to use semantic variants
- `ImguiMapEditor/UI/PreferencesDialog.cpp` — added bounds check for theme search in `renderEditorTab`
- `ImguiMapEditor/UI/Ribbon/Utils/RibbonUtils.h` — added `<format>` include, updated separator color to use explicit alpha transparency
The reason will be displayed to describe this comment to others. Learn more.
📋 Review Summary
The PR successfully introduces a semantic color system and refactors several UI components to use it. This improves maintainability and theme consistency.
🔍 General Feedback
Standardizing UI states (MODIFIED, SAVED, DANGER, etc.) is a great architectural improvement.
Found a few duplicate namespace aliases and opportunities for better clamping in helper functions.
Ensure that removing fallback button styles doesn't lead to regressions in different themes.
The reason will be displayed to describe this comment to others. Learn more.
🟡 The "inactive" state button styling (previously standard gray) has been removed here. If this is intentional to rely on the theme's default button color, it's fine, but ensure it doesn't look out of place when the theme itself is dark.
The reason will be displayed to describe this comment to others. Learn more.
🟡 These helpers should ideally clamp the resulting color components to the [0.0, 1.0] range to avoid potential issues with ImGui color processing or extreme values if the amount is high.
The reason will be displayed to describe this comment to others. Learn more.
🟡 This local color constant could be replaced with a derivative of SemanticColors::MODIFIED to maintain the unified semantic system introduced in this PR.
namespace SC = SemanticColors; is already declared at Line 9; redeclaring it at Line 11 causes a redefinition compile error.
Proposed fix
-namespace SC = SemanticColors;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp` at line 11, Remove the
duplicate namespace alias declaration for SC in RecentMapsPanel.cpp: locate the
second occurrence of "namespace SC = SemanticColors;" (duplicate of the earlier
declaration) and delete it so SC is only aliased once; ensure no other code
relies on multiple aliases and that the single alias remains at the original
declaration site.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ImguiMapEditor/UI/Core/Theme.h`:
- Around line 38-43: The Lighten and Darken functions can produce out-of-range
RGB values; update ImGuiMapEditor::Theme::Lighten and ::Darken to clamp each RGB
channel to the [0.0f, 1.0f] range (preserve c.w) before constructing the
returned ImVec4, e.g., use constexpr std::clamp for c.x/c.y/c.z with amount
added/subtracted; ensure the clamp header is available.
---
Duplicate comments:
In `@ImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp`:
- Line 11: Remove the duplicate namespace alias declaration for SC in
RecentMapsPanel.cpp: locate the second occurrence of "namespace SC =
SemanticColors;" (duplicate of the earlier declaration) and delete it so SC is
only aliased once; ensure no other code relies on multiple aliases and that the
single alias remains at the original declaration site.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 27244df8-2930-4ad2-8452-ea6aa6aceccf
📥 Commits
Reviewing files that changed from the base of the PR and between 7d94a34 and 0e2a3ef.
Added `std::clamp` to theme light/dark helpers to prevent color values from exceeding the valid [0.0, 1.0] range and removed duplicate namespace aliases.
- `ImguiMapEditor/UI/Core/Theme.h` — clamped RGB components in `Lighten` and `Darken` helpers to 0.0-1.0 range
- `ImguiMapEditor/UI/Dialogs/Startup/AvailableClientsPanel.cpp` — removed duplicate `namespace SC` alias
- `ImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp` — [minor] added duplicate `namespace SC` alias
The reason will be displayed to describe this comment to others. Learn more.
📋 Review Summary
The PR successfully introduces a centralized SemanticColors system and updates most UI components to use it, which significantly improves maintainability and theme consistency. However, a critical ImGui style stack imbalance was introduced in StartupDialog.cpp which could lead to crashes or rendering glitches. There are also some minor code duplications (namespace aliases).
🔍 General Feedback
Great work on standardizing the colors. Moving away from hardcoded ImVec4 values makes the application much easier to theme. Please address the ImGui stack imbalance in the startup dialog as it's a regression. Also, consider cleaning up the redundant namespace aliases in RecentMapsPanel.cpp.
The reason will be displayed to describe this comment to others. Learn more.
🔴 ImGui Stack Imbalance: This logic will cause a style color stack underflow or overflow. If ignore_signatures_ changes state during the ImGui::Button call, the PopStyleColor will either be skipped (when it should have happened) or called (when it shouldn't have).
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp` around lines 9 - 13,
Remove the duplicate namespace alias declarations: keep a single declaration of
"namespace SC = SemanticColors;" and delete the other repeated lines so only one
namespace alias remains in RecentMapsPanel.cpp (i.e., remove the extra
occurrences of the symbol "namespace SC = SemanticColors;").
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp`:
- Around line 9-13: Remove the duplicate namespace alias declarations: keep a
single declaration of "namespace SC = SemanticColors;" and delete the other
repeated lines so only one namespace alias remains in RecentMapsPanel.cpp (i.e.,
remove the extra occurrences of the symbol "namespace SC = SemanticColors;").
ℹ️ Review info⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f12445d8-33ba-4344-b5a0-40b71cae8f53
📥 Commits
Reviewing files that changed from the base of the PR and between 0e2a3ef and 955d42e.
Removed redundant namespace aliases in the recent maps panel and refactored the ignore signatures toggle to use a stable boolean state for style application, ensuring consistent color popping.
- `ImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp` — removed redundant `namespace SC` aliases [minor]
- `ImguiMapEditor/UI/Dialogs/Startup/StartupDialog.cpp` — cached `ignore_signatures_` state in `pushed_color` to guarantee matching push and pop calls for button styles
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The refactor successfully introduces a centralized semantic color system and theme management. Static analysis indicates the implementation adheres to quality standards, with no new issues or complexity increases detected.
However, the scope of the changes—spanning more than 20 files—presents a high risk for visual regressions or inconsistent styling. The primary concern is the absence of automated test coverage for critical user-facing features: theme persistence across sessions and the real-time update of UI components during theme selection. Verification of the theme pointer injection into application preferences is recommended to ensure stability.
About this PR
Critical paths for theme selection persistence and live UI updates are currently missing automated verification. This logic should be covered by unit or integration tests to prevent future regressions in preference handling.
The refactor affects a significant number of files. Ensure that visual consistency has been manually verified across all updated dialogs and panels, as the current test suite does not appear to cover the new semantic mappings.
Test suggestions
Verify that selecting a new theme in the Preferences dialog updates the UI colors in real-time (live preview).
Verify that the selected theme is correctly saved and restored upon application restart.
Confirm that the SemanticColors namespace correctly maps semantic roles to the expected color values for each theme.
Verify that specific UI elements, such as the ribbon separator and MainWindow tabs, use the correct semantic constants (e.g., MUTED, SAVED).
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that selecting a new theme in the Preferences dialog updates the UI colors in real-time (live preview).
2. Verify that the selected theme is correctly saved and restored upon application restart.
3. Confirm that the SemanticColors namespace correctly maps semantic roles to the expected color values for each theme.
4. Verify that specific UI elements, such as the ribbon separator and MainWindow tabs, use the correct semantic constants (e.g., MUTED, SAVED).
The reason will be displayed to describe this comment to others. Learn more.
📋 Review Summary
This PR successfully implements a semantic color system and a theme selection mechanism. It centralizes UI state colors (like modified, saved, and danger) into a unified namespace, which greatly improves consistency and maintainability across the codebase.
Key Improvements:
Introduced SemanticColors namespace to standardize state indicators.
Added 7 diverse themes (Tibia RPG, Modern Dark, etc.) with a live preview in Preferences.
Refactored multiple UI components to use theme-aware constants instead of hardcoded values.
🔍 General Feedback
The overall architecture is sound and leverages C++20 features (like std::ranges) effectively. The transition to a theme-driven UI is well-executed.
I have raised some minor points regarding:
Header Hygiene: Removal of unnecessary imgui_internal.h.
Modern C++: Using inline constexpr for global constants in headers to avoid multiple definitions.
Consistency: Ensuring semantically correct constants are used even when values overlap.
Code Quality: Fixing some minor indentation and switch-case issues.
Overall, great work on standardizing the UI visuals!
Simplified theme system by removing redundant themes and consolidated UI text color usage through semantic helpers to improve maintainability and consistency.
- `ImguiMapEditor/UI/Core/Theme.h` — removed unused theme functions, consolidated theme enum, and added `TextPrimary()`/`TextDim()` helpers
- `ImguiMapEditor/UI/Dialogs/Startup/AvailableClientsPanel.cpp` — migrated hardcoded colors to semantic text helpers
- `ImguiMapEditor/UI/Dialogs/Startup/ClientInfoPanel.cpp` — migrated hardcoded colors to semantic text helpers
- `ImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp` — added hover tooltip for map path, migrated colors to semantic helpers
- `ImguiMapEditor/UI/Dialogs/Startup/SelectedMapPanel.cpp` — migrated hardcoded colors to semantic text helpers
- `ImguiMapEditor/UI/Dialogs/Startup/StartupDialog.cpp` — migrated hardcoded colors to semantic text helpers
- `ImguiMapEditor/UI/PreferencesDialog.cpp` — updated theme selection icon logic for streamlined theme list
- `ImguiMapEditor/UI/Ribbon/Panels/ThemePanel.cpp` — updated theme icon mapping to reflect consolidated theme set
The reason will be displayed to describe this comment to others. Learn more.
📋 Review Summary
This PR introduces a robust Semantic Color System that significantly improves the maintainability and consistency of the UI. By centralizing UI state indicators (modified, saved, danger, etc.) into the SemanticColors namespace, the application is now much better equipped for future theming work.
🔍 General Feedback
Semantic Colors: Great architectural improvement. Using descriptive names like SAVED and DANGER instead of raw RGB values makes the code much more readable.
Theme Awareness: The refactoring of StartupDialog and other components to use theme-driven tokens correctly removes hardcoded assumptions about the background colors.
Breaking Changes: The reordering and removal of themes in ThemeType will affect existing user configurations. This is the primary concern that should be addressed (either via migration or by preserving enum values).
Code Quality: The implementation is clean and follows the project's existing style.
🔴 Critical Issues
None (aside from the breaking change in configuration which is 🟠).
🟠 Major Issues
Backward Compatibility: ThemeType enum reordering will cause theme mismatches for existing users.
🟡 Minor Issues
Feature Removal: Several themes were removed without notice in the PR description.
Hardcoded Values: A few hardcoded alpha values remain in the pulse animations.
Overall, this is a very high-quality contribution that moves the project in the right direction.
The reason will be displayed to describe this comment to others. Learn more.
🟢 Excellent Refactor: Replacing hardcoded ImVec4 values with semantic constants is a great step towards a more maintainable and consistent UI. The SemanticColors namespace provides a clear vocabulary for UI states.
The reason will be displayed to describe this comment to others. Learn more.
🟠 Breaking Change: The ThemeType enum has been reordered and several values removed. Since these are likely persisted in user configuration files as integers, this will cause existing settings to load incorrect themes.
For example:
Old ModernDark (1) will now load DocumentLight (1).
Old TibiaRPG (0) will now load ModernDark (0).
Consider preserving the original integer values to maintain backward compatibility, or implement a migration path.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Feature Regression: Several themes (TibiaRPG, ForestGreen, SunsetOrange, etc.) have been removed. While the new semantic color system is a significant improvement, these removals should be explicitly mentioned in the PR description as they affect user-facing features.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Hardcoded Alpha: While the color components are now semantic, the alpha value 0.7f remains hardcoded. It might be worth adding a constant to SemanticColors for standard overlay alphas to ensure consistency across the application.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Area: UIChanges to the wxWidgets GUI (palettes, dialogs, menus, toolbar).Type: EnhancementNew feature or request.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
Type: Enhancement
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
Implemented a unified
SemanticColorsnamespace to standardize UI state indicators (modified, saved, danger, etc.) across the application. Replaced hardcodedImVec4color values throughout dialogs and panels with these semantic constants to ensure theme consistency.ImguiMapEditor/Application.cpp— injected theme pointer into preferencesImguiMapEditor/Presentation/MainWindow.cpp— updated tab coloring using semantic constantsImguiMapEditor/UI/Core/Theme.h— definedSemanticColorsnamespace with standard state/text rolesImguiMapEditor/UI/Dialogs/ClientConfiguration/ClientConfigurationDialog.cpp— replaced hardcoded colors withSemanticColorsconstantsImguiMapEditor/UI/Dialogs/ClientConfiguration/ClientPropertyEditor.cpp— updated color constants to semantic mappingImguiMapEditor/UI/Dialogs/ConfirmationDialog.cpp— included theme header [minor]ImguiMapEditor/UI/Dialogs/EditTownsDialog.cpp— replaced local color definitions withSemanticColorsImguiMapEditor/UI/Dialogs/MapCompatibilityPopup.cpp— used semantic constants for warningsImguiMapEditor/UI/Dialogs/NewMapDialog.cpp— updated label/text coloring and button stylesImguiMapEditor/UI/Dialogs/Properties/ItemPropertiesDialog.cpp— included theme header [minor]ImguiMapEditor/UI/Dialogs/Startup/AvailableClientsPanel.cpp— applied semantic colors to panel headers and statusImguiMapEditor/UI/Dialogs/Startup/ClientInfoPanel.cpp— updated client info status and label stylingImguiMapEditor/UI/Dialogs/Startup/RecentMapsPanel.cpp— applied semantic colors to list items and labelsImguiMapEditor/UI/Dialogs/Startup/SelectedMapPanel.cpp— updated status and label stylingImguiMapEditor/UI/Dialogs/Startup/StartupDialog.cpp— refactored startup dialog styling to be theme-awareImguiMapEditor/UI/Dialogs/UnsavedChangesModal.cpp— included theme header [minor]ImguiMapEditor/UI/Panels/BrushSizePanel.cpp— usedSemanticColors::SAVEDfor active toggleImguiMapEditor/UI/Panels/NewMapPanel.cpp— updated frame/border styling with semantic colorsImguiMapEditor/UI/PreferencesDialog.cpp— added theme selection UI and semantic color usageImguiMapEditor/UI/PreferencesDialog.h— added theme pointer trackingImguiMapEditor/UI/Ribbon/Utils/RibbonUtils.h— updated separator color toSemanticColors::MUTEDImguiMapEditor/UI/Widgets/TilesetGridWidget.cpp— included theme header [minor]ImguiMapEditor/UI/Windows/BrowseTile/BrowseTileWindow.cpp— used semantic constants for property colorsImguiMapEditor/UI/Windows/BrowseTile/SpawnCreatureRenderer.cpp— included theme header [minor]ImguiMapEditor/UI/Windows/IngameBoxWindow.cpp— replaced hardcoded colors with semantic indicatorsSummary by CodeRabbit
New Features
Style