General tab: center Trim Now, 2x Preferences inset, swap Accounts/Video tabs#15
Merged
Merged
Conversation
…inset General tab UI polish: - Log File Trimming: the Trim Now button now floats centered in the gap between the "MB" label and the "Async trim..." hint, with equal padding on both sides. New Bars.SplitCentered (AutoSize | Percent100 | AutoSize) three-section helper — Anchor.None centring, zero pixel literals, so the inset is proportional at 100/125/150%. - Preferences: EQ Client Settings / Process Manager inset doubled 12->24px from the card edges. - Tab order is now General, Accounts, Video, PiP, Hotkeys, Paths (Accounts moved next to General so the common General->Accounts hop is one tab over). Swapped the Tab* constant VALUES + the matching TabPages.Add order; the switch/prewarm/_tabShells all key off the named constants so they follow automatically. Updated the literal tab indices in TrayManager.ShowSettings (the private constants aren't visible cross-file) and made the lazy-save test derive its indices from the constants via reflection so a future reorder can't silently mislabel it again. Verified: dotnet build clean (0/0), dpi-lint clean (no new findings), --test-dpi-baseline PASS, --test-lazy-save PASS, offscreen render of the General tab correct at both 100% and 150%.
📝 WalkthroughWalkthroughThe PR reorders Settings tabs to place Accounts immediately after General (shifting Video to the next slot), adds a new ChangesSettings Tab Reordering with Layout Enhancement
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@UI/DiagRender.cs`:
- Around line 304-305: The reflection calls that read SettingsForm's TabVideo
and TabAccounts constants (via typeof(SettingsForm).GetField(..., sflags) and
GetRawConstantValue()) must be validated before using them to index _tabBuilt:
ensure GetField(...) does not return null, that GetRawConstantValue() yields an
int, and that the int is within _tabBuilt's valid range; if any check fails,
fall back to a safe default index or skip accessing _tabBuilt and log a clear
diagnostic error. Update the code around the GetField usage for TabVideo and
TabAccounts to perform null/type/bounds checks and handle errors gracefully
instead of unguarded casting and indexing.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aa92fb9e-563b-4269-b00a-fe28c43315f5
📒 Files selected for processing (4)
UI/CardLayout.csUI/DiagRender.csUI/SettingsForm.csUI/TrayManager.cs
Comment on lines
+304
to
+305
| int idxVideo = (int)typeof(SettingsForm).GetField("TabVideo", sflags)!.GetRawConstantValue()!; | ||
| int idxAccounts = (int)typeof(SettingsForm).GetField("TabAccounts", sflags)!.GetRawConstantValue()!; |
There was a problem hiding this comment.
Validate reflected tab constants before indexing _tabBuilt.
At Line 304 and Line 305, GetField(... )! + direct casts assume both constants exist and map to valid _tabBuilt indices. If either constant name changes or index drifts, this can crash the diagnostic run before your setup checks execute (affecting Lines 310/340/397/412).
Suggested hardening
var sflags = System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static;
- int idxVideo = (int)typeof(SettingsForm).GetField("TabVideo", sflags)!.GetRawConstantValue()!;
- int idxAccounts = (int)typeof(SettingsForm).GetField("TabAccounts", sflags)!.GetRawConstantValue()!;
+ var fVideo = typeof(SettingsForm).GetField("TabVideo", sflags);
+ var fAccounts = typeof(SettingsForm).GetField("TabAccounts", sflags);
+ if (fVideo?.GetRawConstantValue() is not int idxVideo ||
+ fAccounts?.GetRawConstantValue() is not int idxAccounts)
+ {
+ Console.Error.WriteLine("LazySave: SETUP INVALID — could not resolve TabVideo/TabAccounts constants via reflection.");
+ form.Dispose();
+ return 2;
+ }
@@
var built = (bool[])typeof(SettingsForm).GetField("_tabBuilt", flags)!.GetValue(form)!;
+ if (idxVideo < 0 || idxVideo >= built.Length || idxAccounts < 0 || idxAccounts >= built.Length)
+ {
+ Console.Error.WriteLine($"LazySave: SETUP INVALID — reflected tab indices out of range (Video={idxVideo}, Accounts={idxAccounts}, tabBuiltLen={built.Length}).");
+ form.Dispose();
+ return 2;
+ }
if (built[idxVideo] || built[idxAccounts])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@UI/DiagRender.cs` around lines 304 - 305, The reflection calls that read
SettingsForm's TabVideo and TabAccounts constants (via
typeof(SettingsForm).GetField(..., sflags) and GetRawConstantValue()) must be
validated before using them to index _tabBuilt: ensure GetField(...) does not
return null, that GetRawConstantValue() yields an int, and that the int is
within _tabBuilt's valid range; if any check fails, fall back to a safe default
index or skip accessing _tabBuilt and log a clear diagnostic error. Update the
code around the GetField usage for TabVideo and TabAccounts to perform
null/type/bounds checks and handle errors gracefully instead of unguarded
casting and indexing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Three small General-tab UI tweaks (per Nate):
General · Accounts · Video · PiP · Hotkeys · Paths. Accounts moved next to General so the common General → Accounts hop is one tab over (was General → Video → Accounts).How
Bars.SplitCentered(left, center, right)helper inCardLayout.cs: a 3-column barAutoSize | Percent 100% | AutoSizewith the center groupAnchor.None-centred in the stretchy middle column. That makes the padding(gap − groupWidth)/2on both sides — mathematically equal — with zero pixel literals, so it's proportional at 100/125/150%.Margin12→24px L/R.Tab*constant values and the matchingTabPages.Addorder together. Everything else (_tabShells[],_tabBuilt[], theEnsureTabBuiltswitch, prewarm) keys off the named constants, so it follows automatically.TrayManager.ShowSettings(...)uses literal indices (the constants areprivate), so "Manage Accounts/Characters/Teams" →1and "Video Settings" →2were updated to match.--test-lazy-saveguard hardcoded_tabBuilt[1]=Video /[2]=Accounts. It now derives the indices from theTab*constants via reflection, so a future reorder can't silently mislabel it.Verification (no VM needed)
dotnet build— clean (0 warnings / 0 errors)dpi-lint— clean (no new findings; pre-existing SystemAware-carveout WARNs only)EQSwitch.exe --test-dpi-baseline— PASS (every Form inherits the 96-DPI baseline)EQSwitch.exe --test-lazy-save— PASS (Video/Accounts build + 18-field round-trip clean after the swap)Coordination note
Built in an isolated worktree branched from
origin/mainwhile another session was actively working onmain(the EQ Client Settings overhaul Phase 0). This PR touches onlyCardLayout.cs,SettingsForm.cs,TrayManager.cs,DiagRender.cs— no overlap with that work, so it should merge cleanly alongside it.Summary by CodeRabbit
Release Notes
Enhancements
Bug Fixes