Skip to content

chagning translations should not crash wheel wizard - #527

Merged
DirkDoes merged 1 commit into
mainfrom
fix/change-translation-crash
Oct 11, 2026
Merged

DirkDoes merged 1 commit into
mainfrom
fix/change-translation-crash

Conversation

@DirkDoes

@DirkDoes DirkDoes commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Purpose of this PR:

How to Test:

What Has Been Changed:

Related Issue Link:

Checklist before merging

  • You have created relevant tests

Summary by CodeRabbit

  • New Features
    • Changing the app language now updates translations in place without replacing the current window or settings page.
    • Player and room counts refresh when the language changes.
  • Bug Fixes
    • Dropdown search results now resize the menu to fit filtered options and empty states, then restore its height when search is cleared.
    • Search focus is retained while filtering, including when results include disabled options.

@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: f318dd08-aee0-4ad8-a70f-466ea28423c4

📥 Commits

Reviewing files that changed from the base of the PR and between ca11a1d and 97b8492.


📒 Files selected for processing (9)
  • WheelWizard.UI.Test/ApplicationCompositionTests.cs
  • WheelWizard.UI.Test/SharedControlTests.cs
  • WheelWizard.UI.Test/WindowCompositionTests.cs
  • WheelWizard/Features/Localization/T.cs
  • WheelWizard/Features/Settings/Views/AppearanceSettings.axaml.cs
  • WheelWizard/Views/Components/Dropdown.cs
  • WheelWizard/Views/Shell/BaseWindow.cs
  • WheelWizard/Views/Shell/Layout.axaml.cs
  • WheelWizard/Views/Shell/MainWindowService.cs

💤 Files with no reviewable changes (1)
  • WheelWizard/Views/Shell/MainWindowService.cs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.



📝 Walkthrough

Walkthrough

Language changes now refresh bound translations without recreating the main window. Dropdown search updates the filtered menu layout and focus behavior.

Changes

In-place language changes

Layer / File(s) Summary
Refresh translations on language changes
WheelWizard/Features/Localization/T.cs, WheelWizard/Views/Shell/Layout.axaml.cs
Translations use a shared binding source that sends a UI-thread property-change notification when the language changes. The layout also refreshes player and room counts.
Apply language changes in place
WheelWizard/Features/Settings/Views/AppearanceSettings.axaml.cs, WheelWizard/Views/Shell/MainWindowService.cs, WheelWizard/Views/Shell/BaseWindow.cs, WheelWizard.UI.Test/ApplicationCompositionTests.cs, WheelWizard.UI.Test/WindowCompositionTests.cs
Appearance settings reload after a successful language update and no longer request a main-window refresh. The refresh method and its interface declaration are removed. Composition tests verify language changes and retention of the existing window and settings page. Other included test fixture and dispatcher edits are formatting-only.

Dropdown search

Layer / File(s) Summary
Update filtered menu layout and focus
WheelWizard/Views/Components/Dropdown.cs, WheelWizard.UI.Test/SharedControlTests.cs
Search key handling updates the layout before moving focus and uses tunnel routing. Search no longer applies a separate menu-height cap. Tests check filtered and empty states, restored height, and retained search focus.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant AppearanceSettings
  participant TranslationSource
  participant UIThread
  User->>AppearanceSettings: Confirm a language change
  AppearanceSettings->>AppearanceSettings: Update the language setting and reload settings
  TranslationSource->>UIThread: Notify bound translations of the language change
Loading

Suggested reviewers: patchzyy


Merge Risk: ⚪ Minimal · up to 97b84

Language changes now refresh translations in place instead of recreating the main window, which removes the crash path. The dropdown search layout and focus changes are covered by tests. No merge-blocking issue was found.

Pre-merge checks | Passed 3 | Failed 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check Warning The description contains only the template headings. It does not explain the purpose, testing steps, changes, or related issue, and the relevant-tests checklist remains unchecked despite test changes … Complete every required section with the PR purpose, test instructions, specific changes, and related issue link or "None". Mark the relevant-tests checklist item as checked if the added or updated tests are the intended coverage.
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check Passed The title identifies the main change: preventing crashes when translations change. It is concise, although it contains a spelling error in "chagning".
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description contains only the template headings. It does not explain the purpose, testing steps, changes, or related issue, and the relevant-tests checklist remains unchecked despite test changes in the pull request.


  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR


🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit watched the words turn bright
As languages changed without a flight
The window stayed, the labels flowed
Search trimmed the menu as results showed
One hop, one test, then home to burrow

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

@DirkDoes
DirkDoes merged commit 157cd9d into main Oct 11, 2026
3 checks passed
@DirkDoes
DirkDoes deleted the fix/change-translation-crash branch October 11, 2026 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant