Minor changes to the flag-input-modal #1975
Conversation
WalkthroughReplaced hardcoded UI text with localized strings, removed button padding and fixed preview sizing in FlagInput, and wrapped the modal search input in a centered padded container with localized placeholder. Changes are presentational/localization-only; runtime behavior and public APIs are unchanged. Changes
Sequence Diagram(s)(omitted — changes are presentational/localization-only and do not modify control flow) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (4)
src/client/FlagInput.ts (2)
75-77: Mind the tap target after removing paddingDropping p-[4px] can shrink the clickable area, especially on mobile. Consider preserving an accessible minimum size and visible focus for keyboard users.
- class="border rounded-lg flex cursor-pointer border-black/30 - dark:border-gray-300/60 bg-white/70 dark:bg-[rgba(55,65,81,0.7)] - " + class="border rounded-lg flex cursor-pointer border-black/30 + dark:border-gray-300/60 bg-white/70 dark:bg-[rgba(55,65,81,0.7)] + min-w-[44px] min-h-[44px] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500"Please sanity-check at 320px width and with keyboard nav.
82-85: Preview can collapse without explicit sizingWith width/height removed, the span may render at 0×0 and the img set to 100% won’t show. Give the container a sensible default that can still be overridden.
- style="display:inline-block; - vertical-align:middle; background:#333; border-radius:6px; - overflow:hidden;" + style="display:inline-block; vertical-align:middle; background:#333; border-radius:6px; overflow:hidden; + width: var(--flag-preview-w, 48px); height: var(--flag-preview-h, 64px);"If you prefer fluid sizing, an alternative is aspect-ratio: 3/4 plus a max-height.
src/client/FlagInputModal.ts (2)
21-24: Localize user-facing stringsTitle "Select flag" should go through your i18n helper (e.g., translateText/t) per the PR checklist. Same for the "Search..." placeholder if not already handled elsewhere.
Also, the input class includes both border-none and border; drop the redundancy:
- class="h-[2rem] border-none border border-gray-300 + class="h-[2rem] border border-gray-300
37-39: Prevent double scroll with the new padded wrapperWith the added p-[1rem] wrapper, the fixed h-[90%] on the grid can cause vertical overflow in smaller viewports. Prefer flex growth over hardcoded height.
- class="flex flex-wrap justify-evenly gap-[1rem] overflow-y-auto overflow-x-hidden h-[90%]" + class="flex flex-wrap justify-evenly gap-[1rem] overflow-y-auto overflow-x-hidden flex-1 min-h-0"Test at 320×568 and ensure the modal body doesn’t create two scrollbars.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/client/FlagInput.ts(1 hunks)src/client/FlagInputModal.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1655
File: resources/maps/giantworldmap/manifest.json:165-174
Timestamp: 2025-07-31T12:03:08.052Z
Learning: In OpenFrontIO draft PRs, flag names in map manifest files may be placeholders that will be updated to match the actual SVG flag file names when the assets are received. The final naming depends on the actual flag SVG files provided, not code naming conventions.
🔇 Additional comments (1)
src/client/FlagInputModal.ts (1)
21-21: UI copy change looks goodRemoving “Modal” from the title to “Select flag” matches the PR objective and reads cleaner.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
resources/lang/en.json (1)
619-622: Align button copy with existing style; drop the exclamation.Most buttons use neutral Title Case (e.g., “Start Game”). Suggest “Select Flag” to match “select_lang.title: Select Language” and keep tone consistent.
"flag_input": { "title": "Select Flag", - "button_title": "Pick a flag!" + "button_title": "Select Flag" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
resources/lang/en.json(1 hunks)src/client/FlagInput.ts(2 hunks)src/client/FlagInputModal.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/client/FlagInput.ts
- src/client/FlagInputModal.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-30T18:11:56.014Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#959
File: resources/maps/GatewayToTheAtlantic.json:124-124
Timestamp: 2025-05-30T18:11:56.014Z
Learning: In OpenFrontIO, flag names in map JSON files can use spaces (like "Amazigh flag") and don't need to follow slug format. Flag assets are stored with matching filenames including spaces (e.g., "Amazigh flag.svg"). Nation names and flag names are not translated/localized in the translation files.
Applied to files:
resources/lang/en.json
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
evanpelle
dismissed
coderabbitai[bot]’s stale review
The merge-base changed after approval.
Boostry123
force-pushed
the
reaf/Flag-Input-Selector
branch
from
b3ca504 to
485149e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/client/index.html (1)
201-201: Align widths within the row; prefer flex-basis over percentage widths.Changing
md:w-[15%]→md:w-[20%]for<flag-input>may make the row uneven atmd+because its siblings still usemd:w-[15%]. This can cause earlier wrapping or crowd theusername-input. Consider making sizing consistent across siblings, or switch to flex-basis + grow to let the name field take the rest cleanly.Apply on this line:
-<flag-input class="w-[20%] md:w-[20%]"></flag-input> +<flag-input class="shrink-0 basis-[20%] md:basis-[20%]"></flag-input>Optional follow-ups outside this line for consistency (no diff provided):
- territory-patterns-modal: add
shrink-0 basis-[20%] md:basis-[20%](or keep 15% everywhere).- news-button: match the same basis as above when visible.
- username-input: use
flex-1 min-w-0instead ofw-fullso it expands and truncates nicely.Please verify at 768–1024px:
- The row does not wrap unexpectedly when
news-buttonis visible/hidden.- The username field keeps enough space and does not overflow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/client/index.html(1 hunks)
|
|
github-project-automation
bot
moved this from Development
to Final Review
in OpenFront Release Management
github-project-automation
bot
moved this from Final Review
to Complete
in OpenFront Release Management
Description:
Noticed things were a bit not centered and title had Modal in it.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
boostry