chore(ui): fix icon inconsistencies when ui is docked, added watermark#1181
Conversation
davo0411
changed the title
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Menu
participant Window
participant IconRenderer
User->>Menu: Open Settings
Menu->>Window: Check docking state
alt Window is docked
Menu->>IconRenderer: Render icons in title bar
Menu->>Window: Draw logo as watermark
else Window is undocked
Menu->>IconRenderer: Render icons as ImageButtons in header
Menu->>Window: Draw logo and title in header
end
Menu->>Window: Show/hide separators and layout elements based on docking
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
davo0411
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Menu.cpp (2)
382-474: Consider extracting constants and simplifying the unified rendering function.While this lambda effectively consolidates icon rendering for both modes, it has grown quite complex. Consider these improvements:
Extract magic numbers to named constants:
static constexpr float ICON_SIZE_DOCKED = 40.0f; static constexpr float ICON_SPACING = 8.0f; static constexpr float RIGHT_MARGIN = 45.0f; static constexpr float PADDING_REDUCTION = 2.0f;Consider splitting into separate functions: The docked and undocked rendering logic are quite different and might benefit from separation for maintainability.
The manual mouse interaction handling for docked mode is well-implemented, but the function's complexity suggests it might be doing too much.
476-508: Consider reducing the watermark size for better subtlety.The watermark implementation is technically sound with correct positioning and aspect ratio calculations. However, using 50% of the content height (
watermarkHeightPercent = 0.50f) might make the watermark too prominent, even with the low alpha value.Consider reducing this to 25-30% for a more subtle background effect:
-const float watermarkHeightPercent = 0.50f; // 25% of content height +const float watermarkHeightPercent = 0.25f; // 25% of content height
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Menu.cpp(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (5)
src/Menu.cpp (5)
306-317: Good approach to handling ImGui docking state detection.Using the previous frame's docking state as an approximation is a smart solution to the chicken-and-egg problem of needing to know docking state before creating the window. The conditional window flags correctly implement the requirement to show title bars only when docked.
324-335: Well-structured icon availability and scaling logic.The separation between action icons and logo display is appropriate, and the texture availability checking ensures graceful fallback behavior. The logarithmic UI scaling using
exp2()is a good choice for smooth UI scaling transitions.
336-381: Excellent data-driven approach to action icon management.The
ActionIconstruct with texture, tooltip, and callback encapsulates icon behavior nicely. The conditional population based on texture availability and the comprehensive tooltips provide both robustness and good user experience. This design makes adding new icons straightforward and maintains separation of concerns.
509-562: Well-structured undocked header layout with robust fallback handling.The table-based layout effectively manages the alignment between title/logo and action icons. The consistent use of UI scaling throughout and the comprehensive fallback handling for missing assets demonstrate good defensive programming. The intentional 25% reduction in icon size for visual balance shows attention to UI design details.
563-653: Excellent preservation of functionality with docking-aware adaptations.The conditional separator and fallback text button logic properly maintains existing functionality while adapting to the new docking-aware system. The error toggle button handling is appropriately gated to only show when not docked, and the fallback text buttons ensure users can still access all functionality even when icons are disabled or unavailable.
|
✅ A pre-release build is available for this PR: |
community-shaders#1181) * Update Menu.cpp * style: 🎨 apply pre-commit.ci formatting Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2 participants
Summary by CodeRabbit