fix: prevent overflow for home page quick links#2059
Conversation
📝 WalkthroughWalkthroughThe HomePageRenderer UI layout was refactored to restructure the Quick Links section from a 3-button manually-positioned layout to a 4-column ImGui columns layout with auto-width buttons. The Discord fallback button width calculation was updated to use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
No actionable suggestions for changed features. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Menu/HomePageRenderer.cpp (1)
158-180: Consider responsive quick-link column countUsing exactly 4 columns fixes overflow, but at small widths it compresses labels heavily. A width-based column count (1..4) keeps the layout usable across window sizes.
Proposed refactor
- ImGui::Columns(4, nullptr, false); + float scale = Util::GetUIScale(); + float minColumnWidth = 150.0f * scale; + float availableWidth = ImGui::GetContentRegionAvail().x; + int columnCount = std::clamp(static_cast<int>(availableWidth / minColumnWidth), 1, 4); + ImGui::Columns(columnCount, "QuickLinksColumns", false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Menu/HomePageRenderer.cpp` around lines 158 - 180, The fixed 4-column layout (ImGui::Columns) causes cramped buttons at narrow widths; compute a responsive column count based on available width (use ImGui::GetContentRegionAvailWidth or GetWindowWidth) and a minimum button width, clamp it between 1 and 4, then call ImGui::Columns(calculatedCols, nullptr, false). Iterate your links and call ImGui::Button(...) / ImGui::NextColumn() in a loop (replace the four repeated Button/NextColumn blocks) so the UI adapts automatically as the window resizes while preserving ImGui::Columns, ImGui::NextColumn, and ImGui::Button usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Menu/HomePageRenderer.cpp`:
- Around line 137-140: Clamp the computed buttonWidth (currently calculated as
DISCORD_BANNER_MIN_WIDTH * scale) to the available window width before centering
so it never exceeds windowSize.x (accounting for any desired horizontal padding)
and ensure the SetCursorPosX call receives a non-negative value; update the
logic around the variables buttonWidth, scale and windowSize used before
ImGui::SetCursorPosX and ImGui::Button("Join Discord Server",
ImVec2(buttonWidth, 0)) so the button fits on narrow windows or high UI scales
while still invoking ShellExecuteA with DISCORD_URL when pressed.
---
Nitpick comments:
In `@src/Menu/HomePageRenderer.cpp`:
- Around line 158-180: The fixed 4-column layout (ImGui::Columns) causes cramped
buttons at narrow widths; compute a responsive column count based on available
width (use ImGui::GetContentRegionAvailWidth or GetWindowWidth) and a minimum
button width, clamp it between 1 and 4, then call ImGui::Columns(calculatedCols,
nullptr, false). Iterate your links and call ImGui::Button(...) /
ImGui::NextColumn() in a loop (replace the four repeated Button/NextColumn
blocks) so the UI adapts automatically as the window resizes while preserving
ImGui::Columns, ImGui::NextColumn, and ImGui::Button usage.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be78203a-78bb-49df-9239-b54458c7ee1a
📒 Files selected for processing (2)
src/Menu/HomePageRenderer.cppsrc/Menu/HomePageRenderer.h
💤 Files with no reviewable changes (1)
- src/Menu/HomePageRenderer.h
|
✅ A pre-release build is available for this PR: |
|
Please make sure you test changes before marking a draft ready to avoid follow-ups like this. |
|
Yeah sort of broke my own rule. I enforce that most frontend changes have screenshots at the bare minimum at my workplace. No excuses really here. |
2 participants
Follow up to #2055.
Sort of haphazard on my end, the button widths were hard-coded to only fit 3 in a row, which means the 4th one I added was causing overflow.
This PR refactors so it uses the column system instead of the computed button widths.
This changes the design behaviour though. Previously the buttons were set to a fixed width and then centered. The new behaviour is akin to flex rows in CSS, where it's column-based and the buttons dynamically grow to fit. This means the buttons can stretch really wide (more than the previous 180px) to fit. While it's pretty easy to re-constrain the button widths (apply a container which constrains the columns), the code simplicity might be preferred here. Though if someone feels strongly about constraining the button widths then I can look into that.
Summary by CodeRabbit