Merged
Conversation
WantToBeeMe
changed the base branch from
dev
to
task/settings-rewrite-in-to-DI
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 33
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
WheelWizard/Views/Patterns/MiiImages/BaseMiiImage.cs (2)
85-85:⚠️ Potential issue | 🟠 Major
async void ReloadImagesswallows exceptions.Any exception thrown after an
awaitinsideReloadImages(e.g., fromMiiImageService.GetImageAsync) will be posted to the synchronization context unhandled, potentially crashing the app silently. Consider converting toasync Taskand surfacing errors through a state property (similar to howVrHistoryGraphexposesErrorMessage), or at minimum wrap the body in atry/catch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Patterns/MiiImages/BaseMiiImage.cs` at line 85, The method ReloadImages is declared async void which swallows exceptions; change its signature to async Task (protected async Task ReloadImages(...)) and either wrap the existing body in a try/catch to capture any exceptions from awaited calls (e.g., MiiImageService.GetImageAsync) and set a view-model/state error property (similar to VrHistoryGraph's ErrorMessage), or rethrow/log the exception so callers can observe it; ensure callers that invoked ReloadImages are updated to await the Task or explicitly fire-and-forget with proper exception handling.
83-90:⚠️ Potential issue | 🟠 Major
_reloadCtsis never disposed when the control is destroyed — resource leak.
ReloadImagesdisposes the previous CTS on each call, but the activeCancellationTokenSourceis never disposed when the control is detached from the visual tree. AddOnDetachedFromVisualTreeto cancel and dispose:🛠️ Proposed fix
+ protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e) + { + base.OnDetachedFromVisualTree(e); + _reloadCts?.Cancel(); + _reloadCts?.Dispose(); + _reloadCts = null; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Patterns/MiiImages/BaseMiiImage.cs` around lines 83 - 90, The active CancellationTokenSource stored in _reloadCts is never disposed when the control is removed, causing a resource leak; add an override of OnDetachedFromVisualTree (or the appropriate detach lifecycle method) in the class that cancels and disposes _reloadCts (i.e., call _reloadCts?.Cancel(); _reloadCts?.Dispose(); _reloadCts = null) to mirror the cleanup done in ReloadImages, ensuring you reference the existing _reloadCts field and keep ReloadImages unchanged.WheelWizard/Views/Patterns/VrHistoryGraph.axaml.cs (2)
515-519: 🧹 Nitpick | 🔵 TrivialInconsistent string slicing — prefer a single idiom.
digits[..4]uses range syntax whiledigits.Substring(4, 4)anddigits.Substring(8, 4)use the older API in the same expression.♻️ Proposed fix
- return $"{digits[..4]}-{digits.Substring(4, 4)}-{digits.Substring(8, 4)}"; + return $"{digits[..4]}-{digits[4..8]}-{digits[8..]}";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Patterns/VrHistoryGraph.axaml.cs` around lines 515 - 519, The code builds a formatted string from the local variable digits using mixed slicing styles; make the slicing consistent by replacing the Substring calls with range syntax (or vice versa) so all three segments use the same idiom (e.g., digits[..4], digits[4..8], digits[8..12]) in the return expression that formats the 12-digit value; update the expression that currently returns $"{digits[..4]}-{digits.Substring(4, 4)}-{digits.Substring(8, 4)}" to use the chosen consistent slicing approach.
17-17: 🧹 Nitpick | 🔵 Trivial
lifetimeDaysconstant should use PascalCase.C# convention requires constants to be PascalCase (
LifetimeDays). The field is referenced on lines 289 and 391.♻️ Proposed fix
- private const int lifetimeDays = 999; + private const int LifetimeDays = 999;Also update references:
- HistoryDaysDropdown.Items.Add(new HistoryDaysOption(999, "Lifetime")); // i think this option could make sense + HistoryDaysDropdown.Items.Add(new HistoryDaysOption(LifetimeDays, "Lifetime"));- DateRangeText = days == lifetimeDays ? $"Lifetime history" : $"Past {days} days ({fromDate} to {toDate})"; + DateRangeText = days == LifetimeDays ? "Lifetime history" : $"Past {days} days ({fromDate} to {toDate})";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Patterns/VrHistoryGraph.axaml.cs` at line 17, Rename the constant lifetimeDays to PascalCase LifetimeDays in the VrHistoryGraph class and update all references to use the new name (e.g., where lifetimeDays is read/used around the graph rendering and lifetime calculations). Change the declaration of the constant and replace every usage (including the references currently at the two spots mentioned) to LifetimeDays so the code follows C# naming conventions and compiles without errors.WheelWizard/Views/Patterns/MiiImages/MiiImageLoader.axaml.cs (1)
14-16:⚠️ Potential issue | 🟠 MajorWrong owner type
MiiCarouselinLowQualitySpeedupPropertyregistration.The generic type argument for
AvaloniaProperty.Register<TOwner, TValue>on Line 14 should beMiiImageLoader, notMiiCarousel. Using the wrong owner type can cause incorrect behavior in Avalonia's property inheritance and style-matching machinery.🐛 Proposed fix
- public static readonly StyledProperty<bool> LowQualitySpeedupProperty = AvaloniaProperty.Register<MiiCarousel, bool>( + public static readonly StyledProperty<bool> LowQualitySpeedupProperty = AvaloniaProperty.Register<MiiImageLoader, bool>( nameof(LowQualitySpeedup) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Patterns/MiiImages/MiiImageLoader.axaml.cs` around lines 14 - 16, The LowQualitySpeedupProperty is registered with the wrong owner type (MiiCarousel); update the AvaloniaProperty registration to use MiiImageLoader as the owner so the property is owned by the correct control. Locate the static field LowQualitySpeedupProperty and change the generic owner type in the AvaloniaProperty.Register<...> call from MiiCarousel to MiiImageLoader, ensuring the nameof(LowQualitySpeedup) and the property type (bool) remain unchanged.WheelWizard/Views/Popups/Generic/TextInputWindow.axaml (1)
22-24:⚠️ Potential issue | 🟡 MinorRemove hardcoded debug
ErrorMessage.
ErrorMessage="hey hello"is a leftover placeholder that will unconditionally display as an error on this input field. It should either be empty or bound/set dynamically via code-behind.🐛 Proposed fix
-<behaviorComp:FeedbackTextBox x:Name="InputField" Watermark="{x:Static lang:Phrases.Placeholder_EnterTextHere}" - ErrorMessage="hey hello" - Margin="0,5,0,0" Variant="Default" /> +<behaviorComp:FeedbackTextBox x:Name="InputField" Watermark="{x:Static lang:Phrases.Placeholder_EnterTextHere}" + Margin="0,5,0,0" Variant="Default" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Popups/Generic/TextInputWindow.axaml` around lines 22 - 24, The FeedbackTextBox control named InputField contains a leftover hardcoded ErrorMessage="hey hello" that causes an unconditional error state; remove the literal ErrorMessage attribute (or set it to an empty string) and instead bind ErrorMessage to a viewmodel property or set it dynamically in the TextInputWindow code-behind (use InputField.ErrorMessage) so errors are shown only when appropriate.WheelWizard/Views/Patterns/ModBrowserListItem.axaml.cs (1)
70-93:⚠️ Potential issue | 🟠 Major
new HttpClient()per template application is a socket-exhaustion risk.
OnApplyTemplatecan be called for each item in a list. Instantiating a newHttpClientevery time (and disposing it immediately viausing) prevents connection reuse and can exhaust the socket pool under load.Additionally, no
Timeoutis set, so a stalled server will hold the request open until the OS-level timeout (~2 min by default).Prefer a shared/static
HttpClientor injectIHttpClientFactoryfrom the DI container, and set an explicit timeout:🔧 Suggested approach
+private static readonly HttpClient _sharedClient = new() { Timeout = TimeSpan.FromSeconds(10) }; + protected override async void OnApplyTemplate(TemplateAppliedEventArgs e) { base.OnApplyTemplate(e); var image = e.NameScope.Find<Image>("ThumbnailImage"); if (image == null || string.IsNullOrWhiteSpace(ImageUrl)) return; try { - using var httpClient = new HttpClient(); - var response = await httpClient.GetAsync(ImageUrl); + var response = await _sharedClient.GetAsync(ImageUrl); response.EnsureSuccessStatusCode(); // ... } catch { // Ignore — no image is acceptable } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Patterns/ModBrowserListItem.axaml.cs` around lines 70 - 93, OnApplyTemplate creates a new HttpClient per call which risks socket exhaustion and has no timeout; change the image-loading code in OnApplyTemplate (the ThumbnailImage/ImageUrl handling) to use a reusable client: either obtain an IHttpClientFactory from DI and call CreateClient(...) or use a shared static HttpClient instance shared by the class, and set an explicit Timeout on that client before calling GetAsync; keep the rest of the fetch/stream logic but remove the per-call "new HttpClient()" usage and ensure proper error handling when using the shared/injected client.WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs (1)
87-103:⚠️ Potential issue | 🟠 MajorFix the unsafe null-check in
UserProfilePage.axaml.cs— it will throwNullReferenceException.The
?? string.Emptyguard inEditorGeneral.axaml.cscorrectly prevents aNullReferenceException. However,UserProfilePage.axaml.cslines 359–360 still uses the unsafe pattern:newName = newName?.Trim(); if (newName.Length is > 10 or < 3) // NRE if newName is nullWhen
newNameisnull,newName?.Trim()returnsnull, then calling.LengththrowsNullReferenceException. Apply the same fix asEditorGeneral.axaml.cs:var trimmedName = newName?.Trim() ?? string.Empty; if (trimmedName.Length is > 10 or < 3)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs` around lines 87 - 103, The code in UserProfilePage.axaml.cs performs newName = newName?.Trim(); then checks newName.Length which can throw when newName is null; mirror the safe pattern used in EditorGeneral.axaml.cs (see ValidateMiiName/ValidateCreatorName) by changing to var trimmedName = newName?.Trim() ?? string.Empty; then use trimmedName.Length in the length check and any subsequent logic instead of newName to avoid a NullReferenceException.WheelWizard/Views/Patterns/MiiBlock.axaml.cs (1)
51-72: 🛠️ Refactor suggestion | 🟠 Major
OnApplyTemplateduplicates the body ofSetupHoverVariant— call the helper instead.Lines 61–64 in
OnApplyTemplateare byte-for-byte identical to lines 79–82 inSetupHoverVariant. The helper exists precisely to avoid this;OnApplyTemplateshould call it.♻️ Proposed fix
protected override void OnApplyTemplate(TemplateAppliedEventArgs e) { base.OnApplyTemplate(e); _miiImageLoader = e.NameScope.Find<MiiImageLoaderWithHover>("PART_MiiImageLoader"); - // Set hover variant immediately if (_miiImageLoader != null) { - // Create hover variant with smile expression - var hoverVariant = MiiImageVariants.MiiBlockProfile.Clone(); - hoverVariant.Name = "MiiBlockProfileHover"; - hoverVariant.Expression = MiiImageSpecifications.FaceExpression.smile; - _miiImageLoader.HoverVariant = hoverVariant; - - // If Mii is already set, trigger reload + SetupHoverVariant(); if (Mii != null) { _miiImageLoader.ReloadBothVariants(); } } }Also applies to: 74-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Patterns/MiiBlock.axaml.cs` around lines 51 - 72, OnApplyTemplate currently contains a byte-for-byte duplicate of SetupHoverVariant; simplify by keeping the _miiImageLoader lookup (e.NameScope.Find<MiiImageLoaderWithHover>("PART_MiiImageLoader")) then call SetupHoverVariant() instead of repeating the hover-variant creation and reload logic. Ensure _miiImageLoader is assigned before calling SetupHoverVariant so the helper can set HoverVariant and call ReloadBothVariants() when Mii != null, and remove the duplicated block that creates hoverVariant and performs ReloadBothVariants() in OnApplyTemplate.WheelWizard/Views/Pages/ModsPage.axaml.cs (2)
85-91: 🧹 Nitpick | 🔵 TrivialCross-class
nameofforPropertyChangedis fragile.
nameof(Mod.IsEnabled)is compared against aPropertyChangedEventArgsraised byModManager. This relies onModManagerpropagating the exact string"IsEnabled"— which currently works, but couplesModsPageto an undocumented contract inModManager's implementation. If the property is moved, renamed, or the propagation changes, this breaks silently.Consider using a shared constant or a strongly-typed event on
ModManagerspecifically for enable-state changes instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Pages/ModsPage.axaml.cs` around lines 85 - 91, The handler OnModsChanged currently compares e.PropertyName to nameof(Mod.IsEnabled), which couples ModsPage to Mod's implementation; change ModManager to expose a stable identifier or event for enable-state changes (e.g., a public const string EnabledPropertyName or a dedicated event like EnabledStateChanged) and update OnModsChanged to check that constant or subscribe to the new strongly-typed event instead of using nameof(Mod.IsEnabled); update references to ModManager.Mods and UpdateEnableAllCheckboxState accordingly so the page reacts to the stable contract rather than a cross-class nameof string.
67-83:⚠️ Potential issue | 🟠 MajorMissing unsubscribe from
ModManager.PropertyChanged— potential event handler leak.
ModManager.PropertyChanged += OnModsChangedis registered in the constructor but never removed (noUnloadedhandler). IfModsPageis instantiated more than once during the app's lifetime (e.g., after navigating away and back), each instance will silently accumulate a live reference on the singletonModManager, preventing GC of old page instances and causing duplicate UI updates.Pattern already used on
RoomsPage(Unloaded += RoomsPage_Unloaded→RRLiveRooms.Instance.Unsubscribe) should be applied here.🐛 Proposed fix
public ModsPage() { InitializeComponent(); DataContext = this; Focusable = true; ModManager.PropertyChanged += OnModsChanged; ModManager.ReloadAsync(); SetModsViewVariant(); // Apply priority edits as soon as the user clicks anywhere outside the textbox. AddHandler(PointerPressedEvent, OnPagePointerPressed, RoutingStrategies.Tunnel, true); // Wire up drag-and-drop pointer tracking PointerMoved += OnDragPointerMoved; PointerReleased += OnDragPointerReleased; PointerCaptureLost += OnDragPointerCaptureLost; + + Unloaded += ModsPage_Unloaded; } +private void ModsPage_Unloaded(object? sender, RoutedEventArgs e) +{ + ModManager.PropertyChanged -= OnModsChanged; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Pages/ModsPage.axaml.cs` around lines 67 - 83, ModsPage registers ModManager.PropertyChanged (and several pointer handlers) in the constructor but never unsubscribes them; add an Unloaded handler (e.g., Unloaded += ModsPage_Unloaded) and implement ModsPage_Unloaded to remove the subscription ModManager.PropertyChanged -= OnModsChanged and detach the pointer handlers added in the ctor (remove the AddHandler(PointerPressedEvent, OnPagePointerPressed, ..., true) registration and unsubscribe PointerMoved -= OnDragPointerMoved, PointerReleased -= OnDragPointerReleased, PointerCaptureLost -= OnDragPointerCaptureLost) so instances can be GC'd and won't receive duplicate events.WheelWizard/Views/Patterns/FeedbackTextBox.axaml.cs (1)
96-103: 🧹 Nitpick | 🔵 TrivialConstructor-time
UpdateValidationStatecall is always a no-op.At construction time,
ErrorMessageandWarningMessageare bothnull(styled properties without defaults returnnullbefore XAML bindings are applied). Both flags evaluate tofalse, so this call unconditionally enters the!hasWarningbranch and exits without modifying any classes. The real initialization is already handled byOnPropertyChangedonce XAML property setters run.Consider removing the call from the constructor to avoid confusion, or add a comment explaining why it's there.
🧹 Proposed cleanup
public FeedbackTextBox() { InitializeComponent(); DataContext = this; InputField.TextChanged += (_, _) => RaiseEvent(new TextChangedEventArgs(TextChangedEvent, this)); - UpdateValidationState(hasError: !string.IsNullOrWhiteSpace(ErrorMessage), hasWarning: !string.IsNullOrWhiteSpace(WarningMessage)); // If there is uses for more other events, then we can always add them }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Patterns/FeedbackTextBox.axaml.cs` around lines 96 - 103, The constructor in FeedbackTextBox calls UpdateValidationState with flags derived from styled properties ErrorMessage and WarningMessage which are null at construction, making that call a no-op; remove the UpdateValidationState(...) call from the FeedbackTextBox() constructor (or replace it with a brief comment) and rely on the existing OnPropertyChanged handling to initialize validation state, ensuring you keep the InputField.TextChanged hookup and do not change UpdateValidationState, ErrorMessage, WarningMessage, or OnPropertyChanged implementations.
♻️ Duplicate comments (2)
WheelWizard/Views/Pages/kitchenSink/KitchenSinkTextStylesPage.axaml.cs (1)
6-6: RedundantSectionTooltip => nulloverride — same as base default.Same pattern as in
KitchenSinkDropdownsPageand other section pages. See that file for the proposed fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Pages/kitchenSink/KitchenSinkTextStylesPage.axaml.cs` at line 6, Remove the redundant override of the SectionTooltip property in KitchenSinkTextStylesPage: the override "public override string? SectionTooltip => null;" duplicates the base class default and should be deleted; locate the SectionTooltip property in the KitchenSinkTextStylesPage class and remove that override (matching the pattern used to clean up other pages such as KitchenSinkDropdownsPage).WheelWizard/Views/Pages/kitchenSink/KitchenSinkInputFieldsPage.axaml.cs (1)
6-6: RedundantSectionTooltip => nulloverride — same as base default.Same pattern as in
KitchenSinkDropdownsPage. See that file for the proposed fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@WheelWizard/Views/Pages/kitchenSink/KitchenSinkInputFieldsPage.axaml.cs` at line 6, Remove the redundant property override in KitchenSinkInputFieldsPage: delete the SectionTooltip property override (public override string? SectionTooltip => null;) since it matches the base default; mirror the change made in KitchenSinkDropdownsPage so no behavior changes occur and ensure no other code relies on this override.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@WheelWizard/Services/ModManager.cs`:
- Around line 116-125: The ModManager is raising PropertyChanged with Mod
property names (e.PropertyName like nameof(Mod.IsEnabled), nameof(Mod.Title),
etc.), which violates INotifyPropertyChanged for the ModManager; update the
ModManager's Mod.PropertyChanged handler so that after calling SaveModsAsync()
it calls OnPropertyChanged(nameof(Mods)) instead of
OnPropertyChanged(e.PropertyName), and make the same change in the other Mod
change handler referenced in the diff (leave SaveModsAsync and the
ModManager.Mods collection logic intact so subscribers like ModsPage still
receive updates for the Mods collection).
In `@WheelWizard/Views/App.axaml.cs`:
- Around line 47-48: The comment above ToolTipBubbleBehavior.Initialize() is
truncated; replace it with a complete sentence that explains what "Behavior
overrides" are and what this specific override does — e.g., state that behavior
overrides are native components used to change or extend control behavior at
runtime, and that ToolTipBubbleBehavior.Initialize() registers or configures the
tooltip behavior for the app. Update the comment text near the
ToolTipBubbleBehavior.Initialize() call to a full, self-contained description
referencing ToolTipBubbleBehavior.Initialize and the purpose of the override.
In `@WheelWizard/Views/Behaviors/ToolTipBubbleBehavior.cs`:
- Around line 55-56: ToolTip.SetServiceEnabled(control, false) disables the
platform tooltip service but the current ToolTipBubbleBehavior only opens/closes
tooltips on pointer events (the handlers around the pointer enter/leave code),
so keyboard-only focus won't show them; update ToolTipBubbleBehavior to also
subscribe to focus events (e.g., control.GotFocus/Focused and
control.LostFocus/Unfocused or corresponding FocusedChanged handlers depending
on control type) and call the same open/close logic used by the pointer handlers
(reuse the existing OpenToolTip/CloseToolTip methods or the same internal
show/hide code), and ensure you unsubscribe in the Detach/cleanup path so
behavior cleanup is symmetric with the pointer event handlers.
- Around line 125-132: OnPointerExited currently calls GetState(control) and
allocates state before checking whether the control actually has a tooltip open;
move the IsOpen check earlier to avoid creating state for controls that never
use tooltips: first call if (!ToolTip.GetIsOpen(control)) return; and only then
call var state = GetState(control) followed by CancelPendingOpen(state). Ensure
you reference the existing methods OnPointerExited, GetState and
CancelPendingOpen when making this change.
In `@WheelWizard/Views/Pages/FriendsPage.axaml`:
- Around line 8-9: Remove the duplicate xmlns alias by deleting the unused
xmlns:behaviorComponent declaration (which duplicates xmlns:patterns for
WheelWizard.Views.Patterns), then update any remaining usages of the old alias
(e.g., behaviorComponent:CurrentUserProfile) to use the migrated alias
(patterns:CurrentUserProfile) so only xmlns:patterns remains and no elements
reference behaviorComponent.
In `@WheelWizard/Views/Pages/FriendsPage.axaml.cs`:
- Around line 237-250: The duplicate-friend warning text in
ValidateFriendCodeWarning is hardcoded; replace the literal "This friend is
already in your list." with a localized resource lookup (e.g., use the app's
localization helper or resource manager) so the message comes from language
resources; update the return to fetch the translated string (referencing
ValidateFriendCodeWarning and the duplicateFriend branch) and add the new key to
the resource files used by the localization system, ensuring callers like
FriendCodeGenerator and GameLicenseService.ActiveCurrentFriends behavior remains
unchanged.
In `@WheelWizard/Views/Pages/HomePage.axaml`:
- Around line 179-182: Remove the three commented-out tooltip attribute lines
(ToolTip.Tip, ToolTip.Placement, ToolTip.ShowDelay) that follow the explanatory
comment in HomePage.axaml; keep the explanatory comment about removing the
tooltip and delete the dead code so the dolphin startup button markup no longer
contains those commented attributes.
In `@WheelWizard/Views/Pages/kitchenSink/KitchenSinkButtonsPage.axaml`:
- Line 74: Fix the typo in the TextBlock on KitchenSinkButtonsPage.axaml by
changing the Text attribute value from "Varients" to "Variants" (update the
TextBlock element with Classes="TinyText" Text="Varients" so it reads
Text="Variants").
- Around line 46-57: The "Text + Icon" demo button is rendering with the icon on
the left because the IsIconLeft property is not set; locate the
components:Button element with Text="Text + Icon" and IconData="{StaticResource
Download}" and set IsIconLeft="False" on that element so the icon appears on the
right (leaving the "Icon + Text" button unchanged).
In `@WheelWizard/Views/Pages/kitchenSink/KitchenSinkDropdownsPage.axaml.cs`:
- Line 6: Remove the redundant override of the SectionTooltip property in
KitchenSinkDropdownsPage (public override string? SectionTooltip => null;) since
the base class KitchenSinkSectionPageBase already returns null; locate and
delete the identical override in KitchenSinkDropdownsPage (and apply the same
removal to other pages like KitchenSinkTextStylesPage,
KitchenSinkInputFieldsPage, KitchenSinkButtonsPage) so the derived classes
inherit the base virtual SectionTooltip implementation instead of shadowing it
with a no-op override.
In `@WheelWizard/Views/Pages/kitchenSink/KitchenSinkIconLabelsPage.axaml.cs`:
- Line 6: Remove the redundant override of SectionTooltip in
KitchenSinkIconLabelsPage: delete the line "public override string?
SectionTooltip => null;" since KitchenSinkSectionPageBase already defines
"public virtual string? SectionTooltip => null;" and the override is a no-op;
ensure no other references depend on the override after removal.
In `@WheelWizard/Views/Pages/kitchenSink/KitchenSinkIconsPage.axaml.cs`:
- Line 6: Remove the redundant SectionTooltip override from KitchenSinkIconsPage
(and other Kitchen Sink pages that simply return null) since the base class
KitchenSinkSectionPageBase already defines public virtual string? SectionTooltip
=> null; — locate the override in the KitchenSinkIconsPage class and delete that
property override (and mirror the same removal in any peer pages) so they
inherit the base implementation instead of shadowing it.
In `@WheelWizard/Views/Pages/kitchenSink/KitchenSinkInputFieldsPage.axaml`:
- Line 5: Rename the XAML namespace alias from "behaviorComp" to "patterns" and
update the UI label text that currently reads "FeedbackTextBox Behavior
Component" to a name reflecting the new organization (e.g., "FeedbackTextBox
Pattern Component" or "FeedbackTextBox"); update the xmlns alias declaration
(the xmlns:behaviorComp attribute) and the Label/TextBlock content that uses
that alias in KitchenSinkInputFieldsPage.axaml so bindings/usages referencing
the alias and the visible label match the current Patterns namespace and naming.
- Around line 1-7: Rename the physical folder from "kitchenSink" to
"KitchenSink" so it matches the declared namespace
WheelWizard.Views.Pages.KitchenSink; update any project file entries or IDE
references if necessary and verify XAML class declarations such as
x:Class="WheelWizard.Views.Pages.KitchenSink.KitchenSinkInputFieldsPage" (and
the other eight KitchenSink pages) still resolve after the folder rename to
avoid case-sensitivity build issues on Linux/CI.
In `@WheelWizard/Views/Pages/kitchenSink/KitchenSinkSectionPageBase.cs`:
- Around line 3-7: The IKitchenSinkSection interface is unused outside
KitchenSinkSectionPageBase; either remove the interface or make generics use it:
if you don't need non-UserControlBase implementations, delete
IKitchenSinkSection and move its properties into KitchenSinkSectionPageBase
(remove references to IKitchenSinkSection); otherwise change KitchenSinkPage's
generic constraint from KitchenSinkSectionPageBase to IKitchenSinkSection so the
interface is actually leveraged and update any creation/usage sites to operate
against the interface rather than the concrete KitchenSinkSectionPageBase.
In `@WheelWizard/Views/Pages/KitchenSinkPage.axaml`:
- Around line 72-74: The XAML contains hard-coded user-facing strings (e.g., the
components:FormFieldLabel Text="Light Background" and TipText="Toggle section
block previews between light (Neutral900) and dark (Neutral950) backgrounds",
plus the strings on lines 79 and 84); move these into the app’s
localization/language resources and replace the literal attributes with resource
bindings (e.g., use StaticResource/x:Static or your IStringLocalizer/Loc
extension) so the FormFieldLabel.Text and TipText (and the other two literals)
reference resource keys instead of inline text; ensure resource keys are added
to the language resource file and referenced consistently in the KitchenSinkPage
(components:FormFieldLabel) XAML.
In `@WheelWizard/Views/Pages/KitchenSinkPage.axaml.cs`:
- Around line 204-212: SectionDefinition.Create<T> currently instantiates a
throwaway new T() (calling InitializeComponent) to read SectionName and
SectionTooltip; change this to avoid constructing the full page by making
SectionName and SectionTooltip static on KitchenSinkSectionPageBase subclasses
(or introduce a lightweight metadata type) and update
SectionDefinition.Create<T> to read those static members (or call a static
metadata accessor) and only use the Func<KitchenSinkSectionPageBase> CreatePage
to construct the instance when needed; update references to
SectionName/SectionTooltip in subclasses to the new static members or metadata
provider and ensure Create<T> returns new SectionDefinition(typeof(T).Name,
T.SectionName, T.SectionTooltip, () => new T()) (or equivalent using the
metadata accessor).
- Around line 70-90: CreateSectionContainer currently returns a Border and also
exposes the same instance via the out parameter sectionBorder, which is
redundant; remove the out parameter from the CreateSectionContainer signature,
have it simply return the Border, and update all callers (notably
BuildAllSections and ShowSingleSection) to stop expecting an out parameter and
to use the returned Border value directly (replace out variable usage with the
return). Ensure you remove any now-unused local variables that only existed to
receive the out value and adjust any downstream code that referenced that
variable to reference the returned Border instead.
- Around line 163-188: ShowSingleSection currently re-creates the page via
section.CreatePage() instead of using the cached container built by
BuildAllSections; change ShowSingleSection to look up the prebuilt container in
_allSectionContainersById by sectionId, retrieve the cached Border/container and
assign its border to _singleSectionBorder (instead of calling
CreateSectionContainer), set SectionContent.Content to a ScrollViewer wrapping
that cached container, and keep the existing visibility toggles and
ApplyBlockBackgroundMode call so the cached control tree is reused across
selections.
In `@WheelWizard/Views/Pages/MiiListPage.axaml`:
- Line 6: The xmlns alias xmlns:behaviorComponent is stale — rename the alias to
xmlns:patterns for the mapping to WheelWizard.Views.Patterns and update all XAML
references (any behaviorComponent:TypeName usages) in this file to
patterns:TypeName so the alias matches the namespace and is consistent with
other files like RoomDetailsPage/ModBrowserListItem; ensure you update any
element or attribute references that use the old behaviorComponent prefix to the
new patterns prefix.
In `@WheelWizard/Views/Pages/ModsPage.axaml.cs`:
- Around line 104-107: Replace the current check in UpdateEnableAllCheckboxState
that uses ModManager.Mods.Select(mod => mod.IsEnabled).Contains(false) with a
clearer All(...) predicate: set EnableAllCheckbox.IsChecked based on
ModManager.Mods.All(mod => mod.IsEnabled). Update the method
UpdateEnableAllCheckboxState to reference EnableAllCheckbox, ModManager.Mods and
the IsEnabled property accordingly so the intent is explicit.
In `@WheelWizard/Views/Patterns/CurrentUserProfile.axaml`:
- Line 3: Update the XML namespace alias in CurrentUserProfile.axaml from
behaviorComponent to patterns to reflect the actual CLR namespace
WheelWizard.Views.Patterns; change the xmlns declaration (currently
xmlns:behaviorComponent="clr-namespace:WheelWizard.Views.Patterns") to
xmlns:patterns="clr-namespace:WheelWizard.Views.Patterns" and update all usages
of the alias (every reference to behaviorComponent on lines where it's used) to
patterns so the alias name matches the namespace.
In `@WheelWizard/Views/Patterns/FeedbackTextBox.axaml`:
- Around line 7-9: The xmlns alias "behaviorComp" is misleading because it
points to WheelWizard.Views.Patterns; rename the XML namespace alias (e.g., to
"patterns" or "patternsNs") wherever declared and update the x:DataType and any
other references that use "behaviorComp" (such as
x:DataType="behaviorComp:FeedbackTextBox") to the new alias (for example
x:DataType="patterns:FeedbackTextBox") so the alias accurately reflects the
WheelWizard.Views.Patterns namespace and avoids confusion.
In `@WheelWizard/Views/Patterns/GridModPanel.axaml.cs`:
- Around line 15-16: The static ConcurrentDictionary s_imageCache currently
holds Bitmap instances for the process lifetime causing unbounded memory growth;
replace it with a bounded thread-safe cache (e.g., a fixed-capacity LRU or
size-limited cache used by GridModPanel) and ensure Bitmap instances are
disposed when evicted; update any places that read/write s_imageCache
(references in GridModPanel methods around the bitmap loading logic) to use the
new cache API so additions check capacity and evict/Dispose() the
least-recently-used Bitmap on overflow.
- Around line 78-85: The MemoryStream created for image decoding (memoryStream)
is not disposed; wrap its lifetime in a using/using-declaration around the copy
and Bitmap creation so the intermediate memory buffer is always disposed after
constructing the Bitmap (ensure you still call bitmap.Dispose() if
s_imageCache.TryAdd(modId, bitmap) returns false). Update the block that creates
memoryStream and the Bitmap (referencing memoryStream, Bitmap,
s_imageCache.TryAdd, modId, bitmap.Dispose) to ensure memoryStream is disposed
immediately after the Bitmap is instantiated.
In `@WheelWizard/Views/Patterns/LeaderboardPodiumCard.axaml`:
- Around line 4-5: Remove the duplicate xmlns alias by deleting the stale
declaration "behaviorComponent" (which points to WheelWizard.Views.Patterns) and
keep the single "patterns" xmlns; then replace any usages of
behaviorComponent:MiiImageLoader with patterns:MiiImageLoader so all references
use the consolidated patterns: alias (check the xmlns lines and the element on
line where behaviorComponent:MiiImageLoader is used).
In `@WheelWizard/Views/Patterns/MiiBlock.axaml`:
- Around line 4-6: Remove the duplicate xmlns mapping by deleting
xmlns:behaviorComponent="clr-namespace:WheelWizard.Views.Patterns" and keep a
single xmlns:patterns="clr-namespace:WheelWizard.Views.Patterns"; then replace
all usages of the old alias (currently referenced at the locations mentioned as
lines 48 and 90) to use patterns: for element/class references and patterns| for
any namespace-prefixed x:Name/markup usages so every reference formerly using
behaviorComponent now uses patterns.
In `@WheelWizard/Views/Patterns/MiiImages/MiiCarousel.axaml`:
- Line 6: Remove the unused XAML namespace alias "behaviorComp" which is
declared but never referenced; open MiiCarousel.axaml, find the
xmlns:behaviorComp declaration and delete that attribute so only the used
namespaces (e.g. components:) remain, ensuring references to AspectGrid and
Button (components:AspectGrid, components:Button) keep working and no other
xmlns attributes are accidentally modified.
In `@WheelWizard/Views/Patterns/MiiImages/MiiImageLoader.axaml`:
- Around line 7-10: The xmlns alias "behaviorComp" is now misleading (it points
to WheelWizard.Views.Patterns) so update the alias and its usage: rename the
xmlns:behaviorComp declaration to something like xmlns:patterns and then change
the x:DataType from behaviorComp:MiiImageLoader to patterns:MiiImageLoader
(ensure the x:Name="Self" remains unchanged); search for the xmlns:behaviorComp
and the x:DataType="behaviorComp:MiiImageLoader" occurrences and replace both to
keep the alias and MiiImageLoader binding consistent.
In `@WheelWizard/Views/Popups/Generic/TextInputWindow.axaml.cs`:
- Line 134: The Submit button is being disabled for warnings because
SubmitButton.IsEnabled is set to "!hasError && !hasWarning"; change the behavior
to make warnings non-blocking by removing "!hasWarning" from the enable guard
(leave SubmitButton.IsEnabled tied only to hasError) or, if you intend warnings
to block, rename or rework the API: either make SetWarningValidation ->
SetBlockingWarningValidation or move blocking validations into
inputValidationFunc (treat them as errors and surface the message via
WarningMessage styled as a warning). Update the logic that evaluates
warningValidationFunc/hasWarning and any documentation/comments to reflect the
chosen contract.
In `@WheelWizard/Views/Popups/MiiManagement/MiiEditorWindow.axaml`:
- Line 6: The xmlns alias "behavior" no longer matches the target namespace
WheelWizard.Views.Patterns; rename the alias to "patterns" in the xmlns
declaration and update all usages in this file (replace behavior:MiiImageLoader
and behavior:MiiCarousel with patterns:MiiImageLoader and patterns:MiiCarousel)
so the alias accurately reflects the namespace and avoids confusion.
In `@WheelWizard/Views/Styles/Styles/MiscStyles.axaml`:
- Around line 82-91: The tooltip tail is hardcoded for Top placement
(PART_ToolTipTail Path with Data="M 0 0 L 12 0 L 6 8 Z" in Grid.Row="1") and
will be wrong for Bottom/Left/Right; update the template so the tail adapts to
the control's Placement: either provide separate Path definitions/positions for
each placement and switch them via VisualStateManager or bind a RenderTransform
(rotate/flip and reposition) on PART_ToolTipTail to the Placement property
(TemplateBinding or a converter) so the Path Data/row aligns correctly for Top,
Bottom, Left, and Right. Ensure the element name PART_ToolTipTail remains and
that hit-testing and Stretch/Fill behavior are preserved.
- Around line 110-133: The Grid with name PART_ToolTipChrome used by the
ToolTip.BubbleAnimateIn and ToolTip.BubbleAnimateOut animations lacks a
RenderTransform, so TranslateTransform.Y animations are ignored; update the
template for Grid#PART_ToolTipChrome to include an explicit RenderTransform (a
TranslateTransform) so the KeyFrame setters (TranslateTransform.Y) in the
BubbleAnimateIn and BubbleAnimateOut styles can affect the control.
---
Outside diff comments:
In `@WheelWizard/Views/Pages/ModsPage.axaml.cs`:
- Around line 85-91: The handler OnModsChanged currently compares e.PropertyName
to nameof(Mod.IsEnabled), which couples ModsPage to Mod's implementation; change
ModManager to expose a stable identifier or event for enable-state changes
(e.g., a public const string EnabledPropertyName or a dedicated event like
EnabledStateChanged) and update OnModsChanged to check that constant or
subscribe to the new strongly-typed event instead of using
nameof(Mod.IsEnabled); update references to ModManager.Mods and
UpdateEnableAllCheckboxState accordingly so the page reacts to the stable
contract rather than a cross-class nameof string.
- Around line 67-83: ModsPage registers ModManager.PropertyChanged (and several
pointer handlers) in the constructor but never unsubscribes them; add an
Unloaded handler (e.g., Unloaded += ModsPage_Unloaded) and implement
ModsPage_Unloaded to remove the subscription ModManager.PropertyChanged -=
OnModsChanged and detach the pointer handlers added in the ctor (remove the
AddHandler(PointerPressedEvent, OnPagePointerPressed, ..., true) registration
and unsubscribe PointerMoved -= OnDragPointerMoved, PointerReleased -=
OnDragPointerReleased, PointerCaptureLost -= OnDragPointerCaptureLost) so
instances can be GC'd and won't receive duplicate events.
In `@WheelWizard/Views/Patterns/FeedbackTextBox.axaml.cs`:
- Around line 96-103: The constructor in FeedbackTextBox calls
UpdateValidationState with flags derived from styled properties ErrorMessage and
WarningMessage which are null at construction, making that call a no-op; remove
the UpdateValidationState(...) call from the FeedbackTextBox() constructor (or
replace it with a brief comment) and rely on the existing OnPropertyChanged
handling to initialize validation state, ensuring you keep the
InputField.TextChanged hookup and do not change UpdateValidationState,
ErrorMessage, WarningMessage, or OnPropertyChanged implementations.
In `@WheelWizard/Views/Patterns/MiiBlock.axaml.cs`:
- Around line 51-72: OnApplyTemplate currently contains a byte-for-byte
duplicate of SetupHoverVariant; simplify by keeping the _miiImageLoader lookup
(e.NameScope.Find<MiiImageLoaderWithHover>("PART_MiiImageLoader")) then call
SetupHoverVariant() instead of repeating the hover-variant creation and reload
logic. Ensure _miiImageLoader is assigned before calling SetupHoverVariant so
the helper can set HoverVariant and call ReloadBothVariants() when Mii != null,
and remove the duplicated block that creates hoverVariant and performs
ReloadBothVariants() in OnApplyTemplate.
In `@WheelWizard/Views/Patterns/MiiImages/BaseMiiImage.cs`:
- Line 85: The method ReloadImages is declared async void which swallows
exceptions; change its signature to async Task (protected async Task
ReloadImages(...)) and either wrap the existing body in a try/catch to capture
any exceptions from awaited calls (e.g., MiiImageService.GetImageAsync) and set
a view-model/state error property (similar to VrHistoryGraph's ErrorMessage), or
rethrow/log the exception so callers can observe it; ensure callers that invoked
ReloadImages are updated to await the Task or explicitly fire-and-forget with
proper exception handling.
- Around line 83-90: The active CancellationTokenSource stored in _reloadCts is
never disposed when the control is removed, causing a resource leak; add an
override of OnDetachedFromVisualTree (or the appropriate detach lifecycle
method) in the class that cancels and disposes _reloadCts (i.e., call
_reloadCts?.Cancel(); _reloadCts?.Dispose(); _reloadCts = null) to mirror the
cleanup done in ReloadImages, ensuring you reference the existing _reloadCts
field and keep ReloadImages unchanged.
In `@WheelWizard/Views/Patterns/MiiImages/MiiImageLoader.axaml.cs`:
- Around line 14-16: The LowQualitySpeedupProperty is registered with the wrong
owner type (MiiCarousel); update the AvaloniaProperty registration to use
MiiImageLoader as the owner so the property is owned by the correct control.
Locate the static field LowQualitySpeedupProperty and change the generic owner
type in the AvaloniaProperty.Register<...> call from MiiCarousel to
MiiImageLoader, ensuring the nameof(LowQualitySpeedup) and the property type
(bool) remain unchanged.
In `@WheelWizard/Views/Patterns/ModBrowserListItem.axaml.cs`:
- Around line 70-93: OnApplyTemplate creates a new HttpClient per call which
risks socket exhaustion and has no timeout; change the image-loading code in
OnApplyTemplate (the ThumbnailImage/ImageUrl handling) to use a reusable client:
either obtain an IHttpClientFactory from DI and call CreateClient(...) or use a
shared static HttpClient instance shared by the class, and set an explicit
Timeout on that client before calling GetAsync; keep the rest of the
fetch/stream logic but remove the per-call "new HttpClient()" usage and ensure
proper error handling when using the shared/injected client.
In `@WheelWizard/Views/Patterns/VrHistoryGraph.axaml.cs`:
- Around line 515-519: The code builds a formatted string from the local
variable digits using mixed slicing styles; make the slicing consistent by
replacing the Substring calls with range syntax (or vice versa) so all three
segments use the same idiom (e.g., digits[..4], digits[4..8], digits[8..12]) in
the return expression that formats the 12-digit value; update the expression
that currently returns $"{digits[..4]}-{digits.Substring(4,
4)}-{digits.Substring(8, 4)}" to use the chosen consistent slicing approach.
- Line 17: Rename the constant lifetimeDays to PascalCase LifetimeDays in the
VrHistoryGraph class and update all references to use the new name (e.g., where
lifetimeDays is read/used around the graph rendering and lifetime calculations).
Change the declaration of the constant and replace every usage (including the
references currently at the two spots mentioned) to LifetimeDays so the code
follows C# naming conventions and compiles without errors.
In `@WheelWizard/Views/Popups/Generic/TextInputWindow.axaml`:
- Around line 22-24: The FeedbackTextBox control named InputField contains a
leftover hardcoded ErrorMessage="hey hello" that causes an unconditional error
state; remove the literal ErrorMessage attribute (or set it to an empty string)
and instead bind ErrorMessage to a viewmodel property or set it dynamically in
the TextInputWindow code-behind (use InputField.ErrorMessage) so errors are
shown only when appropriate.
In `@WheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.cs`:
- Around line 87-103: The code in UserProfilePage.axaml.cs performs newName =
newName?.Trim(); then checks newName.Length which can throw when newName is
null; mirror the safe pattern used in EditorGeneral.axaml.cs (see
ValidateMiiName/ValidateCreatorName) by changing to var trimmedName =
newName?.Trim() ?? string.Empty; then use trimmedName.Length in the length check
and any subsequent logic instead of newName to avoid a NullReferenceException.
---
Duplicate comments:
In `@WheelWizard/Views/Pages/kitchenSink/KitchenSinkInputFieldsPage.axaml.cs`:
- Line 6: Remove the redundant property override in KitchenSinkInputFieldsPage:
delete the SectionTooltip property override (public override string?
SectionTooltip => null;) since it matches the base default; mirror the change
made in KitchenSinkDropdownsPage so no behavior changes occur and ensure no
other code relies on this override.
In `@WheelWizard/Views/Pages/kitchenSink/KitchenSinkTextStylesPage.axaml.cs`:
- Line 6: Remove the redundant override of the SectionTooltip property in
KitchenSinkTextStylesPage: the override "public override string? SectionTooltip
=> null;" duplicates the base class default and should be deleted; locate the
SectionTooltip property in the KitchenSinkTextStylesPage class and remove that
override (matching the pattern used to clean up other pages such as
KitchenSinkDropdownsPage).
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (110)
WheelWizard/Services/ModManager.csWheelWizard/Views/App.axamlWheelWizard/Views/App.axaml.csWheelWizard/Views/Behaviors/ToolTipBubbleBehavior.csWheelWizard/Views/Components/AspectGrid.axamlWheelWizard/Views/Components/AspectGrid.axaml.csWheelWizard/Views/Components/Badge.axamlWheelWizard/Views/Components/Badge.axaml.csWheelWizard/Views/Components/Button.axamlWheelWizard/Views/Components/Button.axaml.csWheelWizard/Views/Components/EmptyPageInfo.axamlWheelWizard/Views/Components/EmptyPageInfo.axaml.csWheelWizard/Views/Components/FormFieldLabel.axamlWheelWizard/Views/Components/FormFieldLabel.axaml.csWheelWizard/Views/Components/IconLabel.axamlWheelWizard/Views/Components/IconLabel.axaml.csWheelWizard/Views/Components/IconLabelButton.axamlWheelWizard/Views/Components/IconLabelButton.axaml.csWheelWizard/Views/Components/LoadingIcon.axamlWheelWizard/Views/Components/LoadingIcon.axaml.csWheelWizard/Views/Components/MemeNumberState.axamlWheelWizard/Views/Components/MemeNumberState.axaml.csWheelWizard/Views/Components/MultiColoredIcon.axamlWheelWizard/Views/Components/MultiColoredIcon.axaml.csWheelWizard/Views/Components/MultiIconRadioButton.axamlWheelWizard/Views/Components/MultiIconRadioButton.axaml.csWheelWizard/Views/Components/OptionButton.axamlWheelWizard/Views/Components/OptionButton.axaml.csWheelWizard/Views/Components/PopupListButton.axamlWheelWizard/Views/Components/PopupListButton.axaml.csWheelWizard/Views/Components/StandardLibrary/FormFieldLabel.axaml.csWheelWizard/Views/Components/StateBox.axamlWheelWizard/Views/Components/StateBox.axaml.csWheelWizard/Views/Components/WheelTrail.axamlWheelWizard/Views/Components/WheelTrail.axaml.csWheelWizard/Views/Layout.axamlWheelWizard/Views/Layout.axaml.csWheelWizard/Views/Pages/FriendsPage.axamlWheelWizard/Views/Pages/FriendsPage.axaml.csWheelWizard/Views/Pages/HomePage.axamlWheelWizard/Views/Pages/HomePage.axaml.csWheelWizard/Views/Pages/KitchenSinkPage.axamlWheelWizard/Views/Pages/KitchenSinkPage.axaml.csWheelWizard/Views/Pages/LeaderboardPage.axamlWheelWizard/Views/Pages/MiiListPage.axamlWheelWizard/Views/Pages/MiiListPage.axaml.csWheelWizard/Views/Pages/ModsPage.axamlWheelWizard/Views/Pages/ModsPage.axaml.csWheelWizard/Views/Pages/RoomDetailsPage.axamlWheelWizard/Views/Pages/RoomsPage.axamlWheelWizard/Views/Pages/SettingsPage.axamlWheelWizard/Views/Pages/SettingsPage.axaml.csWheelWizard/Views/Pages/UserProfilePage.axamlWheelWizard/Views/Pages/kitchenSink/KitchenSinkButtonsPage.axamlWheelWizard/Views/Pages/kitchenSink/KitchenSinkButtonsPage.axaml.csWheelWizard/Views/Pages/kitchenSink/KitchenSinkDropdownsPage.axamlWheelWizard/Views/Pages/kitchenSink/KitchenSinkDropdownsPage.axaml.csWheelWizard/Views/Pages/kitchenSink/KitchenSinkIconLabelsPage.axamlWheelWizard/Views/Pages/kitchenSink/KitchenSinkIconLabelsPage.axaml.csWheelWizard/Views/Pages/kitchenSink/KitchenSinkIconsPage.axamlWheelWizard/Views/Pages/kitchenSink/KitchenSinkIconsPage.axaml.csWheelWizard/Views/Pages/kitchenSink/KitchenSinkInputFieldsPage.axamlWheelWizard/Views/Pages/kitchenSink/KitchenSinkInputFieldsPage.axaml.csWheelWizard/Views/Pages/kitchenSink/KitchenSinkSectionPageBase.csWheelWizard/Views/Pages/kitchenSink/KitchenSinkStateBoxesPage.axamlWheelWizard/Views/Pages/kitchenSink/KitchenSinkStateBoxesPage.axaml.csWheelWizard/Views/Pages/kitchenSink/KitchenSinkTextStylesPage.axamlWheelWizard/Views/Pages/kitchenSink/KitchenSinkTextStylesPage.axaml.csWheelWizard/Views/Pages/kitchenSink/KitchenSinkToggleButtonsPage.axamlWheelWizard/Views/Pages/kitchenSink/KitchenSinkToggleButtonsPage.axaml.csWheelWizard/Views/Patterns/CurrentUserProfile.axamlWheelWizard/Views/Patterns/CurrentUserProfile.axaml.csWheelWizard/Views/Patterns/FeedbackTextBox.axamlWheelWizard/Views/Patterns/FeedbackTextBox.axaml.csWheelWizard/Views/Patterns/FriendsListItem.axamlWheelWizard/Views/Patterns/FriendsListItem.axaml.csWheelWizard/Views/Patterns/GridModPanel.axamlWheelWizard/Views/Patterns/GridModPanel.axaml.csWheelWizard/Views/Patterns/LeaderboardPodiumCard.axamlWheelWizard/Views/Patterns/LeaderboardPodiumCard.axaml.csWheelWizard/Views/Patterns/MiiBlock.axamlWheelWizard/Views/Patterns/MiiBlock.axaml.csWheelWizard/Views/Patterns/MiiImages/BaseMiiImage.csWheelWizard/Views/Patterns/MiiImages/MiiCarousel.axamlWheelWizard/Views/Patterns/MiiImages/MiiCarousel.axaml.csWheelWizard/Views/Patterns/MiiImages/MiiImageLoader.axamlWheelWizard/Views/Patterns/MiiImages/MiiImageLoader.axaml.csWheelWizard/Views/Patterns/MiiImages/MiiImageLoaderWithHover.axamlWheelWizard/Views/Patterns/MiiImages/MiiImageLoaderWithHover.axaml.csWheelWizard/Views/Patterns/ModBrowserListItem.axamlWheelWizard/Views/Patterns/ModBrowserListItem.axaml.csWheelWizard/Views/Patterns/PlayerListItem.axamlWheelWizard/Views/Patterns/PlayerListItem.axaml.csWheelWizard/Views/Patterns/SidebarRadioButton.axamlWheelWizard/Views/Patterns/SidebarRadioButton.axaml.csWheelWizard/Views/Patterns/VrHistoryGraph.axamlWheelWizard/Views/Patterns/VrHistoryGraph.axaml.csWheelWizard/Views/Popups/AddFriendConfirmationWindow.axamlWheelWizard/Views/Popups/Generic/TextInputWindow.axamlWheelWizard/Views/Popups/Generic/TextInputWindow.axaml.csWheelWizard/Views/Popups/MiiManagement/MiiCarouselWindow.axamlWheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axamlWheelWizard/Views/Popups/MiiManagement/MiiEditor/EditorGeneral.axaml.csWheelWizard/Views/Popups/MiiManagement/MiiEditorWindow.axamlWheelWizard/Views/Popups/MiiManagement/MiiSelectorWindow.axaml.csWheelWizard/Views/Popups/ModManagement/ModBrowserWindow.axamlWheelWizard/Views/Popups/PlayerProfileWindow.axamlWheelWizard/Views/Styles/Resources/Icons.axamlWheelWizard/Views/Styles/Styles/MiscStyles.axamlWheelWizard/Views/Styles/Styles/TextBoxStyles.axaml
💤 Files with no reviewable changes (2)
- WheelWizard/Views/Components/StandardLibrary/FormFieldLabel.axaml.cs
- WheelWizard/Views/Pages/HomePage.axaml.cs
Comment on lines
+7
to
10
| xmlns:behaviorComp="clr-namespace:WheelWizard.Views.Patterns" | ||
| xmlns:components="clr-namespace:WheelWizard.Views.Components" | ||
| xmlns:conv="clr-namespace:WheelWizard.Views.Converters" | ||
| x:DataType="behaviorComp:MiiImageLoader" x:Name="Self" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Stale alias name behaviorComp — rename to match its new target namespace.
The alias behaviorComp now maps to WheelWizard.Views.Patterns, but the name still implies BehaviorComponent. Since it's used exclusively for x:DataType on Line 10, renaming it (e.g., to patterns) removes the ambiguity introduced by the migration.
✏️ Proposed rename
- xmlns:behaviorComp="clr-namespace:WheelWizard.Views.Patterns"
+ xmlns:patterns="clr-namespace:WheelWizard.Views.Patterns"
xmlns:components="clr-namespace:WheelWizard.Views.Components"
xmlns:conv="clr-namespace:WheelWizard.Views.Converters"
- x:DataType="behaviorComp:MiiImageLoader" x:Name="Self"
+ x:DataType="patterns:MiiImageLoader" x:Name="Self"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| xmlns:behaviorComp="clr-namespace:WheelWizard.Views.Patterns" | |
| xmlns:components="clr-namespace:WheelWizard.Views.Components" | |
| xmlns:conv="clr-namespace:WheelWizard.Views.Converters" | |
| x:DataType="behaviorComp:MiiImageLoader" x:Name="Self" | |
| xmlns:patterns="clr-namespace:WheelWizard.Views.Patterns" | |
| xmlns:components="clr-namespace:WheelWizard.Views.Components" | |
| xmlns:conv="clr-namespace:WheelWizard.Views.Converters" | |
| x:DataType="patterns:MiiImageLoader" x:Name="Self" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@WheelWizard/Views/Patterns/MiiImages/MiiImageLoader.axaml` around lines 7 -
10, The xmlns alias "behaviorComp" is now misleading (it points to
WheelWizard.Views.Patterns) so update the alias and its usage: rename the
xmlns:behaviorComp declaration to something like xmlns:patterns and then change
the x:DataType from behaviorComp:MiiImageLoader to patterns:MiiImageLoader
(ensure the x:Name="Self" remains unchanged); search for the xmlns:behaviorComp
and the x:DataType="behaviorComp:MiiImageLoader" occurrences and replace both to
keep the alias and MiiImageLoader binding consistent.
|
|
||
| SubmitButton.IsEnabled = validationResultError == null; | ||
| InputField.ErrorMessage = validationResultError ?? ""; | ||
| SubmitButton.IsEnabled = !hasError && !hasWarning; |
There was a problem hiding this comment.
Warnings disable the Submit button, making them semantically identical to errors.
!hasWarning in the IsEnabled expression means any warning produced by warningValidationFunc prevents submission. The conventional meaning of a "warning" is informational-but-non-blocking. Callers who invoke SetWarningValidation will reasonably expect the user can still submit — which is the pattern visible in the PR screenshot (yellow alert visible alongside an active Submit button).
If the intent is to block duplicate-friend submission on the friends page, consider either:
- Keeping the block but renaming the API to reflect it (
SetBlockingWarningValidation, or treating it as a second error channel), or - Removing
!hasWarningfrom theIsEnabledguard so warnings display without blocking submission.
💡 Option A — non-blocking warning (user can submit despite warning)
- SubmitButton.IsEnabled = !hasError && !hasWarning;
+ SubmitButton.IsEnabled = !hasError;💡 Option B — keep blocking, but surface intent through the WarningMessage
If blocking is truly the desired behavior for the friends duplicate case, consider routing it through inputValidationFunc as an error with a "warning"-styled error message, or documenting the blocking contract on SetWarningValidation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| SubmitButton.IsEnabled = !hasError && !hasWarning; | |
| SubmitButton.IsEnabled = !hasError; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@WheelWizard/Views/Popups/Generic/TextInputWindow.axaml.cs` at line 134, The
Submit button is being disabled for warnings because SubmitButton.IsEnabled is
set to "!hasError && !hasWarning"; change the behavior to make warnings
non-blocking by removing "!hasWarning" from the enable guard (leave
SubmitButton.IsEnabled tied only to hasError) or, if you intend warnings to
block, rename or rework the API: either make SetWarningValidation ->
SetBlockingWarningValidation or move blocking validations into
inputValidationFunc (treat them as errors and surface the message via
WarningMessage styled as a warning). Update the logic that evaluates
warningValidationFunc/hasWarning and any documentation/comments to reflect the
chosen contract.
| xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" | ||
| xmlns:components="clr-namespace:WheelWizard.Views.Components" | ||
| xmlns:behavior="clr-namespace:WheelWizard.Views.BehaviorComponent" | ||
| xmlns:behavior="clr-namespace:WheelWizard.Views.Patterns" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider renaming the behavior alias to match the new namespace.
The alias xmlns:behavior now points to WheelWizard.Views.Patterns. The name behavior is a vestige of the old BehaviorComponent namespace and could mislead readers. Consider renaming it to patterns for consistency with the target namespace.
♻️ Suggested rename
- xmlns:behavior="clr-namespace:WheelWizard.Views.Patterns"
+ xmlns:patterns="clr-namespace:WheelWizard.Views.Patterns"Then update all usages in this file (behavior:MiiImageLoader → patterns:MiiImageLoader, behavior:MiiCarousel → patterns:MiiCarousel).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@WheelWizard/Views/Popups/MiiManagement/MiiEditorWindow.axaml` at line 6, The
xmlns alias "behavior" no longer matches the target namespace
WheelWizard.Views.Patterns; rename the alias to "patterns" in the xmlns
declaration and update all usages in this file (replace behavior:MiiImageLoader
and behavior:MiiCarousel with patterns:MiiImageLoader and patterns:MiiCarousel)
so the alias accurately reflects the namespace and avoids confusion.
Comment on lines
+110
to
+133
| <Style Selector="ToolTip.BubbleAnimateIn /template/ Grid#PART_ToolTipChrome"> | ||
| <Style.Animations> | ||
| <Animation Duration="0:0:0.04" IterationCount="1" FillMode="Forward"> | ||
| <KeyFrame Cue="0%"> | ||
| <Setter Property="TranslateTransform.Y" Value="11"/> | ||
| </KeyFrame> | ||
| <KeyFrame Cue="100%"> | ||
| <Setter Property="TranslateTransform.Y" Value="0"/> | ||
| </KeyFrame> | ||
| </Animation> | ||
| </Style.Animations> | ||
| </Style> | ||
| <Style Selector="ToolTip.BubbleAnimateOut /template/ Grid#PART_ToolTipChrome"> | ||
| <Style.Animations> | ||
| <Animation Duration="0:0:0.04" IterationCount="1" FillMode="Forward"> | ||
| <KeyFrame Cue="0%"> | ||
| <Setter Property="TranslateTransform.Y" Value="0"/> | ||
| </KeyFrame> | ||
| <KeyFrame Cue="100%"> | ||
| <Setter Property="TranslateTransform.Y" Value="11"/> | ||
| </KeyFrame> | ||
| </Animation> | ||
| </Style.Animations> | ||
| </Style> |
There was a problem hiding this comment.
TranslateTransform.Y animation will silently not play — PART_ToolTipChrome has no RenderTransform set.
Avalonia requires RenderTransform to be set explicitly on a control for transform property animations to work; without it, the animation is silently skipped. The Grid#PART_ToolTipChrome in the template (lines 71–93) has no RenderTransform declared, so BubbleAnimateIn and BubbleAnimateOut will have no visible effect.
🐛 Proposed fix — add an explicit `TranslateTransform` to the template grid
<Grid x:Name="PART_ToolTipChrome" RowDefinitions="Auto,Auto">
+ <Grid.RenderTransform>
+ <TranslateTransform />
+ </Grid.RenderTransform>
<Border Grid.Row="0"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@WheelWizard/Views/Styles/Styles/MiscStyles.axaml` around lines 110 - 133, The
Grid with name PART_ToolTipChrome used by the ToolTip.BubbleAnimateIn and
ToolTip.BubbleAnimateOut animations lacks a RenderTransform, so
TranslateTransform.Y animations are ignored; update the template for
Grid#PART_ToolTipChrome to include an explicit RenderTransform (a
TranslateTransform) so the KeyFrame setters (TranslateTransform.Y) in the
BubbleAnimateIn and BubbleAnimateOut styles can affect the control.
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
2 participants
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.
Important
This branch is based on the rewrite settings branch. since i was not so excited for the potential merge-conflicts that it could cause if i didn't.
Therefore, this branch can not be merged so long the settings rewrite is not merged
Before
After
New
added a new
warningstate to the behaviorTextBox. also imidiatly implemented it in the friends pageSummary by CodeRabbit
Release Notes
New Features
UI/UX Improvements
Bug Fixes