feat: added more support for features in windows#191
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 38 minutes and 59 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReworks several Windows platform handlers: adds a native Grid-based AutoComplete platform view and interface-based handler with mappers, introduces async Windows icon injection, converts several handlers to async image injection, and replaces signature canvas drawing with polyline-based strokes and Windows image export. Changes
Sequence Diagram(s)sequenceDiagram
participant Handler as Handler
participant IconInjector as WindowsIconInjector
participant Resolver as ImageResolver
participant PlatformView as PlatformView (FrameworkElement)
participant Grid as Grid (visual tree)
participant Button as Injected Button
Handler->>IconInjector: InjectAsync(PlatformView, ImageSource, alignment, size, padding, onTap)
IconInjector->>PlatformView: If not loaded, attach Loaded handler (queue)
IconInjector->>Resolver: GetPlatformImageAsync(ImageSource)
Resolver-->>IconInjector: WinUI ImageSource or null
IconInjector->>Grid: Find first child Grid in visual tree
IconInjector->>Grid: Remove previous injected Button (if any)
IconInjector->>Button: Create Button + Image (if image available) and set alignment/padding
Button-->>Grid: Add to specified column (or remove if null)
IconInjector-->>Handler: Injection complete / removed (or skipped on epoch change)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 16
🧹 Nitpick comments (2)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs (2)
127-130: Silent no-op mappers hide unsupported features.
MapThreshold,MapUpdateTextOnSelect, andMapSuggestionListWidthare empty stubs. Consumers setting these properties on Windows will get no behavior and no warning — the comments here are invisible to API users. Consider at least aDebug.WriteLine/Trace.TraceWarningon first use per property, or documenting the Windows limitations on theIFreakyAutoCompleteViewmembers themselves so expectations are set at the API surface.Also applies to: 152-155, 233-236
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs` around lines 127 - 130, The no-op mappers MapThreshold, MapUpdateTextOnSelect, and MapSuggestionListWidth in FreakyAutoCompleteViewHandler silently hide unsupported behavior on Windows; modify each mapper (in class FreakyAutoCompleteViewHandler) to emit a one-time warning (e.g., Trace.TraceWarning or Debug.WriteLine) the first time the mapper is invoked (use a static bool per mapper like s_thresholdWarned, s_updateTextOnSelectWarned, s_suggestionListWidthWarned to ensure it logs only once), include the property name and short explanation that Windows AutoSuggestBox does not support that feature, and also add brief XML documentation on the corresponding IFreakyAutoCompleteView members indicating the Windows limitation so API consumers see it at the surface.
261-266: Reflection-based member access on every item, every keystroke.
FormatTypeusesGetType().GetProperty(memberPath)per-item;UpdateItemsSourcemaps the entire collection through it, andOnSuggestionChosencalls it on every pick. For largeItemsSourcecollections this is measurable. Cache thePropertyInfoper(Type, memberPath)pair, or compile a delegate viaExpression/DynamicMethodif this shows up in profiling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs` around lines 261 - 266, FormatType is doing reflection on every call which is expensive; change it to first look up a cached accessor for the pair (instance.GetType(), memberPath) and use that instead of calling GetType().GetProperty(...) each time. Implement a static cache keyed by (Type, string) that stores either the PropertyInfo or a compiled delegate (Func<object, string>) and have FormatType retrieve from the cache (populate it on first use) and call the cached accessor; update callers like UpdateItemsSource and OnSuggestionChosen to keep using FormatType but benefit from the cache. Optionally prefer compiling an Expression/DynamicMethod delegate for better perf and thread-safety in the cache population.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs`:
- Around line 56-72: ConnectHandler can race with an ongoing async
MapImageSource causing a continuation to call PlatformView.SetIcon on a
disconnected view; add a handler-scoped CancellationTokenSource field (e.g.,
_cts) created/reset in ConnectHandler, pass its token into the async
MapImageSource (change any async void MapImageSource to return Task), observe
token and return early if cancelled, and in DisconnectHandler cancel and dispose
the _cts before calling base.DisconnectHandler so any in-flight image load won’t
call handler.PlatformView.SetIcon after disconnect; also null-check
handler.PlatformView (FreakyWindowsAutoCompleteView.PlatformView) before setting
the icon.
- Around line 91-96: OnSuggestionChosen currently sets sender.Text when
VirtualView.UpdateTextOnSelect is true but raises
VirtualView.RaiseSuggestionChosen afterward, so event listeners see the old
VirtualView.Text; to fix, compute the formatted string via
FormatType(args.SelectedItem, VirtualView.TextMemberPath), assign it to
VirtualView.Text (so the virtual model is up-to-date) before setting sender.Text
(if still needed) and then call VirtualView.RaiseSuggestionChosen(new
FreakyAutoCompleteViewSuggestionChosenEventArgs(args.SelectedItem)); ensure you
update VirtualView.Text rather than relying on the mapper tick or two-way
binding.
- Around line 207-231: MapImageSource is an async void property mapper that can
throw unhandled exceptions and causes race conditions; change the mapper to a
synchronous Action that kicks off an async worker (e.g., StartImageLoadAsync) on
the handler instead of using async void in MapImageSource, add try/catch around
GetRequiredService/GetImageAsync/SetIcon with logging, and guard against stale
results by using a per-handler CancellationTokenSource (cancel and replace on
each mapper call) or a monotonically-increasing epoch/token checked before
calling handler.PlatformView.SetIcon; also extract command-only updates into a
separate MapImageCommand mapper that updates the tap callback without
re-resolving the image (reference MapImageSource, handler.PlatformView.SetIcon,
provider.GetImageSourceService, service.GetImageAsync, and introduce a
handler-scoped CancellationTokenSource or epoch and a StartImageLoadAsync helper
invoked from the synchronous mapper).
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyDatePickerHandler.cs`:
- Around line 9-25: Remove the early null-return in
FreakyDatePickerHandler.HandleAndAlignImageSourceAsync and instead ensure
WindowsIconInjector can handle clearing icons: either change
WindowsIconInjector.InjectAsync to accept a nullable MauiImageSource and perform
removal when imageSource is null, or add a WindowsIconInjector.ClearIconAsync
method and call that when entry.ImageSource is null; update
FreakyDatePickerHandler (and similarly FreakyTimePickerHandler,
FreakyPickerHandler, FreakyEntryHandler) to call the new behavior so toggling
ImageSource off clears any previously injected icon and toggling on still
injects as before.
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyEntryHandler.cs`:
- Around line 26-38: HandleAndAlignImageSourceAsync currently returns early when
entry.ImageSource is null, skipping cleanup and leaving orphaned native icon
buttons; update WindowsIconInjector.InjectAsync to accept a nullable
MauiImageSource (or add WindowsIconInjector.RemoveAsync) and implement the
removal/idempotent logic there (reuse the same cleanup logic found in
WindowsIconInjector.Inject/InjectAsync's removal code), then change
HandleAndAlignImageSourceAsync to always call
WindowsIconInjector.InjectAsync(PlatformView, entry.ImageSource, ...) even when
entry.ImageSource is null so injected icons are cleared; apply the same
nullable-InjectAsync pattern to FreakyTimePickerHandler, FreakyPickerHandler,
and FreakyDatePickerHandler to ensure consistent cleanup across handlers.
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyPickerHandler.cs`:
- Around line 9-25: HandleAndAlignImageSourceAsync currently returns early when
entry.ImageSource is null, preventing cleanup code in
WindowsIconInjector.InjectAsync/Inject() from removing previously injected icon
buttons; add a new WindowsIconInjector.RemoveAsync method that performs the
explicit cleanup of any injected icon/button (same cleanup logic used in
Inject/InjectAsync) and call WindowsIconInjector.RemoveAsync(...) from
HandleAndAlignImageSourceAsync when entry.ImageSource is null so previously
injected icons are removed when the source is cleared.
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs`:
- Around line 138-145: OnClearRequested currently clears PlatformView.Children,
_strokes, _currentStroke and then always calls VirtualView?.OnCleared(), which
causes OnCleared to fire when import/restore flows (OnPointsSpecified,
OnStrokesSpecified) call it; change Clear logic to accept a suppress/raise flag
and only invoke VirtualView?.OnCleared when the call intends to signal a
user-driven clear. Concretely: refactor OnClearRequested into a private
Clear(bool raiseEvent) or add an optional parameter to OnClearRequested (e.g.,
OnClearRequested(object? sender, EventArgs e, bool raiseEvent = true)), move
PlatformView.Children.Clear(), _strokes.Clear(), _currentStroke = null,
_isDrawing = false into that method, and call VirtualView?.OnCleared() only when
raiseEvent is true; then update callers OnPointsSpecified and OnStrokesSpecified
(and the other call sites around the ranges noted) to call the clear routine
with raiseEvent = false when replacing/restoring data.
- Around line 189-191: OnImageStreamRequested currently calls
RenderToStreamAsync with only e.ImageFormat, ignoring the
ImageConstructionSettings in e.Settings; update OnImageStreamRequested (handling
ImageStreamRequestedEventArgs) to pass the full ImageConstructionSettings object
(e.Settings) along with the requested format into RenderToStreamAsync so the
Windows handler honors size, scale, stroke/fill colors like the Apple and
Android handlers do (use the same parameter shape as other platforms and ensure
RenderToStreamAsync signature/overload accepts ImageConstructionSettings).
- Around line 201-202: Add the missing using for the WinRT buffer extension so
pixelBuffer.ToArray() resolves: import
System.Runtime.InteropServices.WindowsRuntime at the top of the file containing
the FreakySignatureViewHandler.win.cs code (where rtb.GetPixelsAsync() and
pixelBuffer.ToArray() are used) so the ToArray() extension method on IBuffer is
available during Windows builds.
- Around line 45-77: Pointer cancellation and capture-loss aren't handled: add
subscriptions for Canvas.PointerCanceled and Canvas.PointerCaptureLost in
ConnectHandler and remove them in DisconnectHandler, implement an
EndCurrentStroke() helper that clears _isDrawing and _currentStroke and invokes
OnStrokeCompleted() as needed, and call EndCurrentStroke() from
OnPointerReleased, OnPointerCanceled and OnPointerCaptureLost so a canceled or
lost pointer ends the stroke and prevents stale drawing state.
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyTimePickerHandler.cs`:
- Around line 9-25: HandleAndAlignImageSourceAsync currently returns early when
entry.ImageSource is null, leaving a previously injected icon and its handler in
the UI; add a public RemoveIcon method to WindowsIconInjector that clears any
existing button tagged "FreakyIconOverlay" from the root grid, then update
HandleAndAlignImageSourceAsync to call
WindowsIconInjector.RemoveIcon(PlatformView) (or equivalent) when
entry.ImageSource is null instead of returning early; keep existing InjectAsync
behavior for non-null sources so re-injection still clears and adds icons.
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/FreakyWindowsAutoCompleteView.cs`:
- Around line 47-63: The icon button currently always receives a Click handler
and remains focusable even when the SetIcon onTap parameter is null; change the
logic in SetIcon so you only attach the Click event handler to _iconButton when
onTap is non-null, and for the null case set properties to disable interaction
(e.g., IsTabStop = false, IsHitTestVisible = false and/or Focusable = false) so
decorative icons are not keyboard-focusable or clickable; ensure
WinGrid.SetColumn(_iconButton, column) still runs and keep the visual setup but
branch the input/interaction wiring based on onTap.
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.cs`:
- Around line 33-37: Get defensive around image service resolution: check that
IPlatformApplication.Current.Services.GetRequiredService<IImageSourceServiceProvider>().GetImageSourceService(mauiImageSource)
is not null before calling GetImageAsync, and wrap the await
service.GetImageAsync(mauiImageSource) in a try/catch that logs the exception
and returns/sets winImageSource to null (or otherwise aborts the injection) so
the async-void caller doesn't crash; reference the GetImageSourceService and
GetImageAsync calls and the mauiImageSource/winImageSource variables when adding
the null-check and try/catch.
- Around line 87-100: The horizontal padding calculation uses Math.Max(width,
height) so totalPad grows when the icon is taller than wide; change the
computation used for the left/right Thickness to use width only (e.g. totalPad =
width + padding * 2) so that platformView.Padding assignments inside the
ImageAlignment.Left and default (Right) cases reserve horizontal space based on
width, and keep iconButton.HorizontalAlignment logic unchanged.
- Around line 59-63: The Loaded handler added in InjectAsync leaks and can be
added multiple times; modify InjectAsync to avoid accumulating handlers by
capturing the RoutedEventHandler (e.g., handler) before subscribing, and have
the handler unsubscribe itself (platformView.Loaded -= handler) immediately upon
running then call Inject(platformView,...); additionally, prevent duplicate
pending handlers by checking a per-platformView flag or Tag (or clear any
existing handler reference) before attaching, and optionally clear that flag on
platformView.Unloaded to allow future re-injection.
- Around line 24-31: The InjectAsync and Inject methods use FrameworkElement for
the platformView parameter but later set platformView.Padding which is only
available on Control; change the parameter type for both InjectAsync and Inject
from FrameworkElement to Control (e.g., method signatures for InjectAsync(...)
and Inject(...)) so platformView.Padding = new Thickness(...) compiles; update
any local references or overloads accordingly—the callers (CalendarDatePicker,
TextBox, TimePicker, etc.) already derive from Control so this type change is
safe.
---
Nitpick comments:
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs`:
- Around line 127-130: The no-op mappers MapThreshold, MapUpdateTextOnSelect,
and MapSuggestionListWidth in FreakyAutoCompleteViewHandler silently hide
unsupported behavior on Windows; modify each mapper (in class
FreakyAutoCompleteViewHandler) to emit a one-time warning (e.g.,
Trace.TraceWarning or Debug.WriteLine) the first time the mapper is invoked (use
a static bool per mapper like s_thresholdWarned, s_updateTextOnSelectWarned,
s_suggestionListWidthWarned to ensure it logs only once), include the property
name and short explanation that Windows AutoSuggestBox does not support that
feature, and also add brief XML documentation on the corresponding
IFreakyAutoCompleteView members indicating the Windows limitation so API
consumers see it at the surface.
- Around line 261-266: FormatType is doing reflection on every call which is
expensive; change it to first look up a cached accessor for the pair
(instance.GetType(), memberPath) and use that instead of calling
GetType().GetProperty(...) each time. Implement a static cache keyed by (Type,
string) that stores either the PropertyInfo or a compiled delegate (Func<object,
string>) and have FormatType retrieve from the cache (populate it on first use)
and call the cached accessor; update callers like UpdateItemsSource and
OnSuggestionChosen to keep using FormatType but benefit from the cache.
Optionally prefer compiling an Expression/DynamicMethod delegate for better perf
and thread-safety in the cache population.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a0b1dca4-4452-4eb3-8072-195818d7b8db
📒 Files selected for processing (14)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyDatePickerHandler.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyEntryHandler.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyPickerHandler.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyTimePickerHandler.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/FreakyWindowsAutoCompleteView.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.csdocs/FreakyAutoCompleteView.mddocs/FreakyDatePicker.mddocs/FreakyEntry.mddocs/FreakyPicker.mddocs/FreakySignatureCanvasView.mddocs/FreakyTimePicker.md
💤 Files with no reviewable changes (4)
- docs/FreakyEntry.md
- docs/FreakyPicker.md
- docs/FreakyDatePicker.md
- docs/FreakyTimePicker.md
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs (1)
82-90:⚠️ Potential issue | 🟡 Minor
DisconnectHandlercancels_ctsbut never disposes it.On disconnect without a subsequent reconnect, the
CancellationTokenSourceallocated inConnectHandler/MapImageSourceis leaked. Minor, but trivially fixable.♻️ Suggested fix
protected override void DisconnectHandler(FreakyWindowsAutoCompleteView platformView) { - _cts.Cancel(); + _cts.Cancel(); + _cts.Dispose(); var asb = platformView.AutoSuggestBox; asb.TextChanged -= OnTextChanged; asb.SuggestionChosen -= OnSuggestionChosen; asb.QuerySubmitted -= OnQuerySubmitted; base.DisconnectHandler(platformView); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs` around lines 82 - 90, DisconnectHandler currently calls _cts.Cancel() but never disposes the CancellationTokenSource, leaking it when there is no reconnect; update DisconnectHandler (and ensure behavior aligns with ConnectHandler/MapImageSource) to cancel and then Dispose() the existing _cts, set _cts to null, and guard with a null check to avoid ObjectDisposedException on repeated disconnects; this ensures the CancellationTokenSource created in ConnectHandler/MapImageSource is properly cleaned up.MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs (1)
139-151:⚠️ Potential issue | 🟡 Minor
OnStrokeCompletednow fires on cancel/capture-lost — consider whether that's intentional.Both
OnPointerCanceledandOnPointerCaptureLostfunnel throughEndCurrentStroke, which unconditionally raisesVirtualView?.OnStrokeCompleted(). Consumers that treat the event as a "user finished a stroke" signal (e.g., autosave, validation, haptic feedback) will now receive it when the OS aborts the gesture. If that's undesired, gate the notification on the release path only.♻️ Suggested fix
- private void OnPointerReleased(object sender, PointerRoutedEventArgs e) - { - if (_isDrawing && sender is Canvas canvas) - canvas.ReleasePointerCapture(e.Pointer); - EndCurrentStroke(); - } - - private void OnPointerCanceled(object sender, PointerRoutedEventArgs e) - => EndCurrentStroke(); - - private void OnPointerCaptureLost(object sender, PointerRoutedEventArgs e) - => EndCurrentStroke(); - - private void EndCurrentStroke() - { - if (!_isDrawing) return; - _isDrawing = false; - _currentStroke = null; - VirtualView?.OnStrokeCompleted(); - } + private void OnPointerReleased(object sender, PointerRoutedEventArgs e) + { + if (_isDrawing && sender is Canvas canvas) + canvas.ReleasePointerCapture(e.Pointer); + EndCurrentStroke(notify: true); + } + + private void OnPointerCanceled(object sender, PointerRoutedEventArgs e) + => EndCurrentStroke(notify: false); + + private void OnPointerCaptureLost(object sender, PointerRoutedEventArgs e) + => EndCurrentStroke(notify: false); + + private void EndCurrentStroke(bool notify) + { + if (!_isDrawing) return; + _isDrawing = false; + _currentStroke = null; + if (notify) + VirtualView?.OnStrokeCompleted(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs` around lines 139 - 151, The EndCurrentStroke currently always calls VirtualView?.OnStrokeCompleted(), causing OnStrokeCompleted to fire on cancellations/capture-loss; change the API so EndCurrentStroke accepts a flag (e.g., completed: bool) or add a separate EndCurrentStrokeAborted method and only invoke VirtualView.OnStrokeCompleted when completed is true; update OnPointerCanceled and OnPointerCaptureLost to call the aborted path (completed=false) while the normal pointer-release handler calls EndCurrentStroke(completed=true) so consumers only get OnStrokeCompleted for user-completed strokes.
🧹 Nitpick comments (2)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs (2)
19-48: RoutingImageCommand/ImageCommandParameterthroughMapImageSourceforces a full image reload on every command change.Every change to
ImageCommandorImageCommandParametercancels the in-flight image load, allocates a newCancellationTokenSource, and re-resolves the platform image — even though those two properties only affect the tap callback closure, not pixels. On initial binding (when all seven image-related properties fire), each mapping tick also supersedes the previous, so the icon can flicker or arrive out of order. Split the command handling into its own no-op-for-image mapper (or rebuild only the tap closure viaFreakyWindowsAutoCompleteView.SetIconwithout re-fetching the image).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs` around lines 19 - 48, The ImageCommand and ImageCommandParameter are currently routed to MapImageSource which forces Cancel/replace of in-flight image loads and causes flicker; change the PropertyMapper so ImageCommand and ImageCommandParameter use a separate mapper (e.g., MapImageCommand or MapImageTap) that does not re-fetch the image but only rebuilds the tap callback, or update FreakyWindowsAutoCompleteView.SetIcon to allow replacing the tap closure without cancelling/creating a new CancellationTokenSource; locate usages in the PropertyMapper and the MapImageSource method to implement the new no-op-for-image mapper (or modify SetIcon) so command/parameter updates do not trigger image reloads.
61-80:MapFont/MapTextAlignmentwill be invoked again by the property mapper — the direct calls inConnectHandlerare redundant.
ViewHandlerwalks thePropertyMapperafterConnectHandlerreturns, which will callMapFontandMapTextAlignmentfor their registered keys. Calling them explicitly here just doubles the work. The other direct assignments (Text,PlaceholderText,IsSuggestionListOpen,IsEnabled,UpdateItemsSource) are similarly covered byMapText,MapPlaceholder, etc. — you can drop them if you trust the mapper, or keep the direct path and drop the mapper entries for the initial-only values. Either way, pick one source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs` around lines 61 - 80, ConnectHandler currently sets properties on the platform view (Text, PlaceholderText, IsSuggestionListOpen, IsEnabled, UpdateItemsSource) and then calls MapFont and MapTextAlignment directly, but the ViewHandler's PropertyMapper will also invoke MapFont/MapTextAlignment after ConnectHandler returns causing duplicate work; to fix, remove the explicit calls to MapFont and MapTextAlignment from ConnectHandler (or alternatively remove their entries from the PropertyMapper) and rely on the mapper to apply those mappings, keeping ConnectHandler only for wiring event handlers (OnTextChanged, OnSuggestionChosen, OnQuerySubmitted) and any platform-only initialization such as CancellationTokenSource handling and UpdateItemsSource if you decide it's not covered by a mapper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs`:
- Around line 220-248: MapImageSource currently fires MapImageSourceAsync as an
unobserved task; change MapImageSourceAsync (and the MapImageSource caller
pattern) to handle exceptions and cancellation: wrap the body of
MapImageSourceAsync in a try/catch that swallows OperationCanceledException and
logs other exceptions, check token.IsCancellationRequested immediately after
awaits, and verify PlatformView is still valid/not disposed before calling
PlatformView.SetIcon; keep the caller behavior using handler._cts but ensure any
exceptions inside MapImageSourceAsync are caught and do not escape to
TaskScheduler.UnobservedTaskException.
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs`:
- Around line 172-192: OnPointsRequested and OnStrokesRequested currently return
deferred projections over the live _strokes and each Polyline.Points collection,
which can mutate after the call; fix by materializing the results immediately:
for OnPointsRequested set e.Points to a concrete collection (e.g., call ToList()
on the SelectMany result) and for OnStrokesRequested build a stable collection
of collections (e.g., project each s.Points into a List<Point> and then ToList()
the outer sequence) so callers receive immutable snapshots rather than lazy
enumerables over Polyline.Points.
- Around line 216-267: RenderToStreamAsync currently mutates
PlatformView.Background (causing a visible flash) and ignores
ImageConstructionSettings.StrokeColor/StrokeWidth; instead, do not touch
PlatformView.Background — render the control to a RenderTargetBitmap
(RenderTargetBitmap.RenderAsync(PlatformView)), then create an off-screen target
(e.g., a WriteableBitmap or SoftwareBitmap sized to rtb.PixelWidth/PixelHeight)
pre-filled with settings.BackgroundColor and composite the RTB pixels onto that
off-screen bitmap (alpha-blend using premultiplied BGRA) before encoding;
additionally, honor settings.StrokeColor and settings.StrokeWidth by re-drawing
the control’s stroke geometry onto the off-screen bitmap (or by transforming the
pixel buffer: replace stroke pixels by sampling/thresholding and painting new
color/stroke thickness) using the control’s stroke data/source (access via
PlatformView’s stroke collection or rendering API) so the exported image
reflects the requested stroke color/width without ever mutating
PlatformView.Background.
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.cs`:
- Around line 55-118: InjectAsync can race: multiple calls await
GetPlatformImageAsync and an earlier completion can overwrite a later injected
button; fix by adding a per-view epoch or cancellation that is checked after the
await. For example, add an epoch int on PendingLoad (or a
ConditionalWeakTable<FrameworkElement, int>) and increment it at the start of
InjectAsync (or when scheduling a new PendingLoad), capture the current epoch
into a local before awaiting GetPlatformImageAsync, then after the await compare
the captured epoch to the current epoch for that platformView and bail (return)
if they differ; alternatively, store a CancellationTokenSource on PendingLoad,
cancel the previous CTS at the start of a new InjectAsync call and pass the
token to GetPlatformImageAsync (or check token after await) and return when
cancelled. Ensure the check is performed before calling Inject(platformView,
...) and before wiring/adding Loaded handlers, and reference PendingLoad,
_pending, InjectAsync, GetPlatformImageAsync, Inject, and platformView when
implementing.
- Around line 160-175: RemoveFromGrid(FrameworkElement) and the corresponding
Inject method assume the template root is the first child and silently skip when
it's not a WinGrid; update both to search the visual tree for the first WinGrid
(e.g., traverse children of platformView using
VisualTreeHelper.GetChildrenCount/GetChild) and operate on that grid, or
alternatively throw a clear diagnostic if no WinGrid is found. Locate the
methods named Inject and RemoveFromGrid(FrameworkElement) and replace the
single-child check with a small walker that finds the first WinGrid (then call
the existing RemoveFromGrid(WinGrid) logic), ensuring callers like
Entry/Picker/TimePicker/DatePicker behave correctly with non-Grid roots. Ensure
behavior preserves current functionality when a WinGrid is present and fails
loudly (exception or log) when none is found.
---
Duplicate comments:
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs`:
- Around line 82-90: DisconnectHandler currently calls _cts.Cancel() but never
disposes the CancellationTokenSource, leaking it when there is no reconnect;
update DisconnectHandler (and ensure behavior aligns with
ConnectHandler/MapImageSource) to cancel and then Dispose() the existing _cts,
set _cts to null, and guard with a null check to avoid ObjectDisposedException
on repeated disconnects; this ensures the CancellationTokenSource created in
ConnectHandler/MapImageSource is properly cleaned up.
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs`:
- Around line 139-151: The EndCurrentStroke currently always calls
VirtualView?.OnStrokeCompleted(), causing OnStrokeCompleted to fire on
cancellations/capture-loss; change the API so EndCurrentStroke accepts a flag
(e.g., completed: bool) or add a separate EndCurrentStrokeAborted method and
only invoke VirtualView.OnStrokeCompleted when completed is true; update
OnPointerCanceled and OnPointerCaptureLost to call the aborted path
(completed=false) while the normal pointer-release handler calls
EndCurrentStroke(completed=true) so consumers only get OnStrokeCompleted for
user-completed strokes.
---
Nitpick comments:
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs`:
- Around line 19-48: The ImageCommand and ImageCommandParameter are currently
routed to MapImageSource which forces Cancel/replace of in-flight image loads
and causes flicker; change the PropertyMapper so ImageCommand and
ImageCommandParameter use a separate mapper (e.g., MapImageCommand or
MapImageTap) that does not re-fetch the image but only rebuilds the tap
callback, or update FreakyWindowsAutoCompleteView.SetIcon to allow replacing the
tap closure without cancelling/creating a new CancellationTokenSource; locate
usages in the PropertyMapper and the MapImageSource method to implement the new
no-op-for-image mapper (or modify SetIcon) so command/parameter updates do not
trigger image reloads.
- Around line 61-80: ConnectHandler currently sets properties on the platform
view (Text, PlaceholderText, IsSuggestionListOpen, IsEnabled, UpdateItemsSource)
and then calls MapFont and MapTextAlignment directly, but the ViewHandler's
PropertyMapper will also invoke MapFont/MapTextAlignment after ConnectHandler
returns causing duplicate work; to fix, remove the explicit calls to MapFont and
MapTextAlignment from ConnectHandler (or alternatively remove their entries from
the PropertyMapper) and rely on the mapper to apply those mappings, keeping
ConnectHandler only for wiring event handlers (OnTextChanged,
OnSuggestionChosen, OnQuerySubmitted) and any platform-only initialization such
as CancellationTokenSource handling and UpdateItemsSource if you decide it's not
covered by a mapper.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27624468-b81b-4610-b4ab-f88f14ac865d
📒 Files selected for processing (17)
MAUI.FreakyControls/MAUI.FreakyControls/Enums/AnimationType.csMAUI.FreakyControls/MAUI.FreakyControls/EventArgs/FreakyAutoCompleteViewTextChangedEventArgs.csMAUI.FreakyControls/MAUI.FreakyControls/Extensions/Extensions.csMAUI.FreakyControls/MAUI.FreakyControls/FreakyCheckbox/FreakyCheckbox.csMAUI.FreakyControls/MAUI.FreakyControls/FreakyJumpList/FreakyJumpList.csMAUI.FreakyControls/MAUI.FreakyControls/FreakySwitch/FreakySwitch.csMAUI.FreakyControls/MAUI.FreakyControls/Maui.FreakyControls.csprojMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyDatePickerHandler.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyEntryHandler.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyPickerHandler.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyTimePickerHandler.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/FreakyWindowsAutoCompleteView.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.csMAUI.FreakyControls/Samples/BaseContentPage.csMAUI.FreakyControls/Samples/Samples.csproj
💤 Files with no reviewable changes (2)
- MAUI.FreakyControls/MAUI.FreakyControls/FreakySwitch/FreakySwitch.cs
- MAUI.FreakyControls/Samples/BaseContentPage.cs
✅ Files skipped from review due to trivial changes (6)
- MAUI.FreakyControls/MAUI.FreakyControls/Enums/AnimationType.cs
- MAUI.FreakyControls/MAUI.FreakyControls/Extensions/Extensions.cs
- MAUI.FreakyControls/MAUI.FreakyControls/Maui.FreakyControls.csproj
- MAUI.FreakyControls/MAUI.FreakyControls/EventArgs/FreakyAutoCompleteViewTextChangedEventArgs.cs
- MAUI.FreakyControls/MAUI.FreakyControls/FreakyCheckbox/FreakyCheckbox.cs
- MAUI.FreakyControls/MAUI.FreakyControls/FreakyJumpList/FreakyJumpList.cs
🚧 Files skipped from review as they are similar to previous changes (5)
- MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyEntryHandler.cs
- MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyTimePickerHandler.cs
- MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyDatePickerHandler.cs
- MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyPickerHandler.cs
- MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/FreakyWindowsAutoCompleteView.cs
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs`:
- Around line 231-349: PaintSegment currently blends by directly scaling source
RGB by "ink" which effectively premultiplies colors and ignores destination
alpha; update the blending to perform proper straight-alpha (source-over)
compositing: compute src_alpha = ink (or ink * strokeAlpha if separated),
dst_alpha = pixels[offset+3]/255f, out_alpha = src_alpha +
dst_alpha*(1-src_alpha), then compute out_rgb = (src_rgb*src_alpha +
dst_rgb*dst_alpha*(1-src_alpha)) / max(out_alpha, eps) and write back
unassociated RGB and out_alpha*255 into pixels[offset..offset+3]; modify the
variables used in PaintSegment (ink, strokeAlpha, sR/sG/sB, pixels, offset)
accordingly and ensure you clamp/divide safely to avoid NaNs when out_alpha is
zero.
- Around line 31-35: The handler uses shared _isDrawing and _currentStroke so
multi-touch corrupts strokes; add a uint? field like _activePointerId to store
PointerRoutedEventArgs.Pointer.PointerId when starting a stroke in
OnPointerPressed, and in OnPointerMoved/OnPointerReleased/OnPointerCanceled
ignore events whose Pointer.PointerId does not match _activePointerId; clear
_activePointerId when the stroke finishes (and only update/_currentStroke when
the incoming pressed event sets the active id). Ensure all references to
_isDrawing and _currentStroke are gated by the active pointer id check so only
the originating pointer can modify that stroke.
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.cs`:
- Around line 42-52: Remove must fully restore the host control state: when
Inject modified the host control's Padding and set the overlay ImageSource,
store the original Padding in the pending state and, in Remove, always restore
that original Padding and clear the overlay's ImageSource (not only when
platformView.IsLoaded). Ensure you still detach any pending Loaded handler
(cancelState.Handler) and remove the entry from _pending so no future Loaded
event recreates the overlay, and call RemoveFromGrid(platformView) or otherwise
clear overlay references even if the control is not currently loaded; update
both the Remove method and the corresponding cleanup path used around the Inject
logic to use the stored original Padding and clear ImageSource unconditionally.
- Around line 181-184: FindFirstGrid currently doesn't consider the root itself
when searching for a WinGrid, causing valid root WinGrid instances to be missed;
update the BFS in FindFirstGrid to check the dequeued node for being a WinGrid
(e.g., test the current DependencyObject for WinGrid and return it) before
iterating/enqueuing its children so the root is included in the search.
- Around line 82-87: The code returns early when winImageSource is null leaving
the previous overlay/padding in place; change the branch so that if
epochBox.Value == capturedEpoch but winImageSource is null you explicitly clear
the old overlay before returning by calling the same removal path (e.g., call
Inject(...) or your overlay removal method with a null/empty image) — locate the
await mauiImageSource.GetPlatformImageAsync(mauiContext) call and the check
using epochBox.Value and capturedEpoch, and ensure when winImageSource is null
and the epoch still matches you invoke Inject (or the removal routine) to remove
the previous icon/padding before returning.
- Around line 89-104: The injected WinButton (created in WindowsIconInjector.cs
with Tag = IconTag and variable button) currently always participates in
hit-testing and tab navigation and always has button.Click subscribed; update
the construction so that if the provided onTap delegate is null you set
button.IsHitTestVisible = false and button.IsTabStop = false and do NOT
subscribe to button.Click, otherwise subscribe to button.Click to invoke onTap;
make these changes around the code that assigns Content = new WinImage { ... }
and the subsequent button.Click subscription to ensure non-interactive
visual-only icons do not receive focus or clicks.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9280b95a-8a23-4d28-a75c-73d25cba556c
📒 Files selected for processing (3)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.cs (2)
112-125:⚠️ Potential issue | 🟠 MajorHandle image-resolution failures before returning to UI callers.
GetPlatformImageAsynccan fault for bad/unsupported image sources; currently that skips the stale-overlay cleanup path and can surface through mapper callers. Keep the epoch check, but catch resolution failures and clear the overlay only if this call is still current.🛡️ Proposed guard
- var winImageSource = (await mauiImageSource.GetPlatformImageAsync(mauiContext))?.Value; + Microsoft.UI.Xaml.Media.ImageSource? winImageSource; + try + { + winImageSource = (await mauiImageSource.GetPlatformImageAsync(mauiContext))?.Value; + } + catch (Exception ex) + { + if (epochBox.Value == capturedEpoch) + { + System.Diagnostics.Debug.WriteLine($"[WindowsIconInjector] Image resolution failed: {ex}"); + Remove(platformView); + } + + return; + } // Bail silently when superseded by a newer InjectAsync call. if (epochBox.Value != capturedEpoch) return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.cs` around lines 112 - 125, Wrap the await mauiImageSource.GetPlatformImageAsync(mauiContext) call in a try/catch that handles image-resolution exceptions: if the call throws, check if epochBox.Value == capturedEpoch and if so call Remove(platformView) to clear any overlay/padding, then return (do not let the exception bubble to mapper callers); keep the existing epoch check logic (epochBox.Value != capturedEpoch) after the awaited call as-is so stale operations still bail without touching UI. Use the existing symbols mauiImageSource.GetPlatformImageAsync, epochBox.Value, capturedEpoch, and Remove(platformView).
195-223:⚠️ Potential issue | 🟠 MajorPreserve the captured padding while the icon is injected.
The current assignments replace the control’s existing top/bottom/opposite-side padding with
0untilRemoveruns, which can visibly change text/control layout while the icon is present.🐛 Proposed padding preservation
if (!state.HasOriginalPadding) { state.OriginalPadding = control.Padding; state.HasOriginalPadding = true; } + var originalPadding = state.OriginalPadding; + switch (alignment) { case ImageAlignment.Left: - control.Padding = new WinThickness(totalPad, 0, 0, 0); + control.Padding = new WinThickness( + originalPadding.Left + totalPad, + originalPadding.Top, + originalPadding.Right, + originalPadding.Bottom); iconButton.HorizontalAlignment = WinHorizontalAlignment.Left; break; default: // Right - control.Padding = new WinThickness(0, 0, totalPad, 0); + control.Padding = new WinThickness( + originalPadding.Left, + originalPadding.Top, + originalPadding.Right + totalPad, + originalPadding.Bottom); iconButton.HorizontalAlignment = WinHorizontalAlignment.Right; break; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.cs` around lines 195 - 223, When injecting the icon in WindowsIconInjector (use symbols state, state.OriginalPadding, state.HasOriginalPadding, control.Padding, totalPad, alignment, ImageAlignment.Left, iconButton.HorizontalAlignment), preserve the control's other padding values instead of zeroing them: on first injection capture state.OriginalPadding as you already do, then when setting control.Padding compute a new WinThickness that uses totalPad for the injected side and uses state.OriginalPadding.Top/Bottom and the original opposite-side value for the other components (e.g. for Left alignment set new padding = (totalPad, state.OriginalPadding.Top, state.OriginalPadding.Right, state.OriginalPadding.Bottom); for Right alignment set new padding = (state.OriginalPadding.Left, state.OriginalPadding.Top, totalPad, state.OriginalPadding.Bottom)). Keep the HorizontalAlignment adjustments for iconButton as-is.
🧹 Nitpick comments (1)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs (1)
86-99: Property mapper changes don't propagate to already-drawn strokes.
MapStrokeColor/MapStrokeWidthonly update the private fields used when constructing newPolylines. Any strokes already on the canvas keep their original brush/thickness, which may surprise consumers who toggleStrokeColor/StrokeWidthat runtime expecting a live restyle. If that's intentional (each stroke records its own style), fine — but worth either documenting or iterating_strokeshere to keep them in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs` around lines 86 - 99, MapStrokeColor and MapStrokeWidth only update handler._strokeColor and handler._strokeThickness for new polylines, leaving already-drawn strokes unchanged; update the existing _strokes collection after setting the new values so live strokes are restyled: in MapStrokeColor, after setting handler._strokeColor, iterate handler._strokes and update each Polyline's Stroke/Brush to use the new WinColor-derived brush; similarly in MapStrokeWidth, after setting handler._strokeThickness iterate handler._strokes and set each Polyline's StrokeThickness to the new value; reference the MapStrokeColor/MapStrokeWidth methods, handler._strokeColor, handler._strokeThickness and the handler._strokes collection when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs`:
- Around line 241-243: The code computes scaleX, scaleY and then uses scaleAvg
for stroke radius (scaleAvg = (scaleX + scaleY) / 2f), which makes stroke
thickness isotropic under non-uniform DesiredSizeOrScale; either
enforce/document uniform scaling or change the rendering to preserve anisotropic
stroke geometry: stop using scaleAvg and instead apply an affine transform or
per-axis scaling when rendering (e.g., use scaleX and scaleY to transform the
canvas or compute separate radii for X/Y), or render to a pre-scaled
RenderTargetBitmap before stroking so round caps remain consistent; update the
logic around scaleX, scaleY, scaleAvg (and any code that computes stroke radius)
to implement one of these fixes and add an assertion/documentation if you choose
to disallow non-uniform scaling.
- Around line 230-239: GetImageStreamAsync can run before PlatformView is laid
out so PlatformView.ActualWidth/ActualHeight may be 0 and later scaleX/scaleY
become enormous; detect this and early-return an empty/background-only image
instead of proceeding. In the method GetImageStreamAsync, after computing
canvasW/canvasH (from PlatformView.ActualWidth/ActualHeight) and before using
settings.DesiredSizeOrScale or computing scaleX/scaleY, check if canvasW <= 1 ||
canvasH <= 1 and _strokes.Count > 0 and if so create and return a clear image
stream (or a stream with just the background) so callers receive a valid image
rather than a silently broken one; alternatively, if you prefer fail-fast
behavior, throw a descriptive exception instead—ensure the branch references
PlatformView, GetImageStreamAsync, settings.DesiredSizeOrScale and _strokes so
the change is easy to locate.
- Around line 156-177: ClearCanvas can leave _activePointerId non-null and lock
input; update ClearCanvas and EndCurrentStroke to clear the active pointer id:
in ClearCanvas(bool raiseEvent) set _activePointerId = null before returning (so
any externally triggered clears release the pointer), and in EndCurrentStroke()
also set _activePointerId = null (already suggested in the comment path where
_isDrawing is set to false) so that OnPointerReleased/OnPointerPressed logic no
longer gets stuck; locate these symbols (ClearCanvas, EndCurrentStroke,
_activePointerId, OnPointerReleased, OnPointerPressed) and ensure both code
paths explicitly null out _activePointerId.
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.cs`:
- Line 226: The iconButton is being added without grid attached properties so it
may land in the wrong cell; update the code around
rootGrid.Children.Add(iconButton) in WindowsIconInjector.cs to first anchor the
button to row 0/column 0 and span all rows/columns of rootGrid (use
Grid.SetRow(iconButton, 0) and Grid.SetColumn(iconButton, 0) and set
Grid.SetRowSpan(iconButton, Math.Max(1, rootGrid.RowDefinitions.Count)) and
Grid.SetColumnSpan(iconButton, Math.Max(1, rootGrid.ColumnDefinitions.Count)))
and only then call rootGrid.Children.Add(iconButton) so the overlay covers the
entire grid.
---
Duplicate comments:
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.cs`:
- Around line 112-125: Wrap the await
mauiImageSource.GetPlatformImageAsync(mauiContext) call in a try/catch that
handles image-resolution exceptions: if the call throws, check if epochBox.Value
== capturedEpoch and if so call Remove(platformView) to clear any
overlay/padding, then return (do not let the exception bubble to mapper
callers); keep the existing epoch check logic (epochBox.Value != capturedEpoch)
after the awaited call as-is so stale operations still bail without touching UI.
Use the existing symbols mauiImageSource.GetPlatformImageAsync, epochBox.Value,
capturedEpoch, and Remove(platformView).
- Around line 195-223: When injecting the icon in WindowsIconInjector (use
symbols state, state.OriginalPadding, state.HasOriginalPadding, control.Padding,
totalPad, alignment, ImageAlignment.Left, iconButton.HorizontalAlignment),
preserve the control's other padding values instead of zeroing them: on first
injection capture state.OriginalPadding as you already do, then when setting
control.Padding compute a new WinThickness that uses totalPad for the injected
side and uses state.OriginalPadding.Top/Bottom and the original opposite-side
value for the other components (e.g. for Left alignment set new padding =
(totalPad, state.OriginalPadding.Top, state.OriginalPadding.Right,
state.OriginalPadding.Bottom); for Right alignment set new padding =
(state.OriginalPadding.Left, state.OriginalPadding.Top, totalPad,
state.OriginalPadding.Bottom)). Keep the HorizontalAlignment adjustments for
iconButton as-is.
---
Nitpick comments:
In
`@MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs`:
- Around line 86-99: MapStrokeColor and MapStrokeWidth only update
handler._strokeColor and handler._strokeThickness for new polylines, leaving
already-drawn strokes unchanged; update the existing _strokes collection after
setting the new values so live strokes are restyled: in MapStrokeColor, after
setting handler._strokeColor, iterate handler._strokes and update each
Polyline's Stroke/Brush to use the new WinColor-derived brush; similarly in
MapStrokeWidth, after setting handler._strokeThickness iterate handler._strokes
and set each Polyline's StrokeThickness to the new value; reference the
MapStrokeColor/MapStrokeWidth methods, handler._strokeColor,
handler._strokeThickness and the handler._strokes collection when making the
change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a558986f-ac6e-4929-8e19-9b2a0fb0602e
📒 Files selected for processing (2)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.csMAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/NativeControls/WindowsIconInjector.cs
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.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation