Skip to content

Conversation

@DevelopingTom
Copy link
Contributor

Description:

Issue:
With low resolution screens, or simply by resizing the window, the build menu can be drawn above the event logs:
image

Changes:

  • use larger responsive Tailwind classes:
fix_resize.mp4

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory
  • I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

IngloriousTom

@DevelopingTom DevelopingTom self-assigned this Oct 9, 2025
@DevelopingTom DevelopingTom requested a review from a team as a code owner October 9, 2025 21:59
@DevelopingTom DevelopingTom added Bug UI/UX UI/UX changes including assets, menus, QoL, etc. labels Oct 9, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

Walkthrough

Responsive CSS utility classes in UnitDisplay.ts were updated to use different breakpoints (adding 2xl and lg, replacing some xl/md usages), adjusting flex direction, gaps, and visibility across screen sizes. No logic, state, or exported API changes.

Changes

Cohort / File(s) Summary
Responsive layout classes
src/client/graphics/layers/UnitDisplay.ts
Updated conditional CSS class list: shifted breakpoints (md/xl → lg/2xl), adjusted flex direction, gaps, and visibility rules. No functional logic changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • evanpelle
  • scottanderson

Poem

Breakpoints shuffle in a gentle dance,
Flex turns, gaps widen—just a new stance.
No state to wrangle, no logic to tame,
Only screens deciding the frame.
Pixels align, 2xl joins in play—
UI whispers: “Resize and sway.”

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the primary change of updating to larger responsive classes to address layout issues and directly reflects the adjustments made to Tailwind breakpoints without introducing unnecessary detail.
Description Check ✅ Passed The description clearly details the low-resolution layout issue and explains the adoption of larger Tailwind breakpoint classes to fix it, supported by screenshots and checklist confirmations for translation, tests, and UI updates.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/client/graphics/layers/UnitDisplay.ts (1)

130-130: Remove redundant 2xl:flex class.

The lg:flex class already makes the element display as flex from 1024px onwards, including at the 2xl breakpoint. The 2xl:flex declaration is redundant and can be removed.

Apply this diff:

-        class="hidden 2xl:flex lg:flex fixed bottom-4 left-1/2 transform -translate-x-1/2 z-[1100] 2xl:flex-row xl:flex-col lg:flex-col 2xl:gap-5 xl:gap-2 lg:gap-2 justify-center items-center"
+        class="hidden lg:flex fixed bottom-4 left-1/2 transform -translate-x-1/2 z-[1100] 2xl:flex-row xl:flex-col lg:flex-col 2xl:gap-5 xl:gap-2 lg:gap-2 justify-center items-center"
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc38fed and 9cbf49c.

📒 Files selected for processing (1)
  • src/client/graphics/layers/UnitDisplay.ts (1 hunks)
🔇 Additional comments (1)
src/client/graphics/layers/UnitDisplay.ts (1)

130-130: Confirm the responsive layout changes fix the overlap issue.

The changes adjust breakpoints and flex directions to address the build menu rendering above event logs. Please verify that:

  • At lg (1024px-1279px): flex-col with gap-2 prevents overlap
  • At xl (1280px-1535px): flex-col with gap-2 prevents overlap
  • At 2xl (1536px+): flex-row with gap-5 prevents overlap

The responsive class updates look correct for addressing layout issues across different screen sizes, assuming the overlap problem occurred at the previous breakpoint configuration.

return html`
<div
class="hidden xl:flex md:flex fixed bottom-4 left-1/2 transform -translate-x-1/2 z-[1100] xl:flex-row lg:flex-col md:flex-col xl:gap-5 lg:gap-2 md:gap-2 justify-center items-center"
class="hidden 2xl:flex lg:flex fixed bottom-4 left-1/2 transform -translate-x-1/2 z-[1100] 2xl:flex-row xl:flex-col lg:flex-col 2xl:gap-5 xl:gap-2 lg:gap-2 justify-center items-center"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify build menu accessibility on tablets and smaller screens.

The build menu is completely hidden below the lg breakpoint (< 1024px), which includes tablets and smaller devices. This means users on tablets (768px-1023px) and mobile devices cannot access the build menu at all.

If this is intentional because an alternative build interface exists for smaller screens, please confirm. Otherwise, consider adding an md:flex class or providing an alternative UI pattern for tablet users.

Run the following script to check if an alternative build menu exists for smaller screens:


🏁 Script executed:

#!/bin/bash
# Description: Search for alternative build menu implementations for mobile/tablet

# Search for mobile-specific build menu components or responsive variants
rg -n --type=ts -A 5 -B 5 'md:flex|mobile.*build|tablet.*build'

# Search for other references to UnitDisplay or build menu
rg -n --type=ts 'UnitDisplay|unit-display'

Length of output: 7330


Make build menu visible on tablets
Add md:flex to the build menu container (currently hidden below lg) so it’s accessible on 768–1023px screens.

🤖 Prompt for AI Agents
In src/client/graphics/layers/UnitDisplay.ts around line 130, the build menu
container is hidden below lg (class starts with "hidden ... lg:flex") which
makes it inaccessible on tablet widths; add the responsive class md:flex to the
class list (e.g., change from "hidden 2xl:flex lg:flex ..." to include
"md:flex") so the menu becomes visible on 768–1023px screens while preserving
existing behavior at other breakpoints.

Copy link
Collaborator

@evanpelle evanpelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@evanpelle evanpelle added this to the v26 milestone Oct 10, 2025
@evanpelle evanpelle merged commit 4f73548 into openfrontio:main Oct 10, 2025
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug UI/UX UI/UX changes including assets, menus, QoL, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants