Fix PopupPanel and PopupMenu menu styles #96518
Merged
Conversation
This file contains 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
KoBeWi
reviewed
Sep 28, 2024
KoBeWi
reviewed
Sep 28, 2024
| @@ -1331,17 +1334,17 @@ void EditorThemeManager::_populate_standard_styles(const Ref<EditorTheme> &p_the | |||
|
|
|||
| Ref<StyleBoxLine> style_popup_separator(memnew(StyleBoxLine)); | |||
| style_popup_separator->set_color(p_config.separator_color); | |||
| style_popup_separator->set_grow_begin(p_config.popup_margin - MAX(Math::round(EDSCALE), p_config.border_width)); | |||
| style_popup_separator->set_grow_end(p_config.popup_margin - MAX(Math::round(EDSCALE), p_config.border_width)); | |||
| style_popup_separator->set_grow_begin(Math::round(EDSCALE) - MAX(Math::round(EDSCALE), p_config.border_width)); | |||
There was a problem hiding this comment.
This looks sus. There is EDSCALE on both sides, so this has high chance of being 0. Though not sure what's the expected value here.
KoBeWi
approved these changes
Sep 28, 2024
KoBeWi
force-pushed
the
popup-panel-style
branch
from
0c75c1f to
549bffd
Compare
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes: #96517
Edit: Fixes: #67657 (border part)
This makes popup menus using PopupPanel and PopupMenus have same base look, as it is in Godot 3.
Panel border
Adding a default border to the PopupPanel to make it easier to differentiate from the background.
I believe this was removed in PR #45607 and then added back for PopupMenus in PR #48655 (also removed support for Border Size on PopupMenus)
Before:
After:
Border color
In Godot 3 both styles of popup menus uses the same border color, light on dark theme and dark border on light themes.
Godot 3 example:
Currently Godot 4 uses dark border for PopupMenus and with Border Size in Editor Settings set to 1 or 2 PopupPanel uses a light border in dark themes.
Before: with border size set
After: border size is set to deafult
Because the PopupMenu border is visible by default I went with that color for both in this PR.
Maybe there could be a editor setting for changing outline color, but that would need to be a proposal in that case.
Border Size
In Godot 3 both PopupPanel and PopupMenus uses the Border Size.
PopupPanel uses this already.
This PR also changes PopupMenus to use the editor setting Border Size for the outer border.
Separator width
Due to the change of using Border Size the separator line in PopupMenus was much more obvious that it overlaps the outer border. So this was also changed
Before:
After:
Compact
Before:
After:
