fix: HandlerNotFoundException while debugging in Windows Machine mode#176
fix: HandlerNotFoundException while debugging in Windows Machine mode#176
Conversation
WalkthroughThis update introduces Windows-specific handler implementations for two custom controls in the MAUI.FreakyControls library. The Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant FreakyAutoCompleteViewHandler
participant AutoSuggestBox
App->>FreakyAutoCompleteViewHandler: Instantiate handler
FreakyAutoCompleteViewHandler->>AutoSuggestBox: CreatePlatformView()
FreakyAutoCompleteViewHandler->>AutoSuggestBox: Attach event handlers (TextChanged, SuggestionChosen, QuerySubmitted)
AutoSuggestBox-->>FreakyAutoCompleteViewHandler: Event (TextChanged/SuggestionChosen/QuerySubmitted)
FreakyAutoCompleteViewHandler-->>App: Handle event (stubbed)
App->>FreakyAutoCompleteViewHandler: Dispose handler
FreakyAutoCompleteViewHandler->>AutoSuggestBox: Detach event handlers
sequenceDiagram
participant App
participant FreakySignatureCanvasViewHandler
participant Canvas
App->>FreakySignatureCanvasViewHandler: Instantiate handler
FreakySignatureCanvasViewHandler->>Canvas: CreatePlatformView()
FreakySignatureCanvasViewHandler->>Canvas: Attach pointer event handlers
Canvas-->>FreakySignatureCanvasViewHandler: PointerPressed/PointerMoved/PointerReleased
FreakySignatureCanvasViewHandler-->>Canvas: Draw line (on move)
App->>FreakySignatureCanvasViewHandler: Dispose handler
FreakySignatureCanvasViewHandler->>Canvas: Detach pointer event handlers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs (1)
15-17: Consider removing the TODO commentThe comment "You can configure it further if needed" appears to be a placeholder that should be removed or replaced with actual configuration if needed.
{ IsSuggestionListOpen = false - // You can configure it further if needed };MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs (1)
46-53: Consider adding null check for PointerRoutedEventArgsThe
OnPointerPressedmethod should include a null check for the event arguments to ensure robustness.private void OnPointerPressed(object sender, PointerRoutedEventArgs e) { + if (e == null) + return; if (sender is not Canvas canvas) return; _isDrawing = true; _previousPoint = e.GetCurrentPoint(canvas).Position; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs(1 hunks)MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
🔇 Additional comments (4)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs (3)
8-19: Class now properly derives from ViewHandler for Windows platformThe implementation correctly inherits from
ViewHandler<FreakyAutoCompleteView, AutoSuggestBox>and overridesCreatePlatformView()to return a new Windows-specificAutoSuggestBoxcontrol. This ensures proper integration with the MAUI handler architecture.
20-28: Proper event wiring in ConnectHandlerThe handler correctly implements the
ConnectHandlermethod to wire up the necessary events for theAutoSuggestBoxcontrol, which is essential for the autocomplete functionality to work properly.
30-38: Good practice - event cleanup in DisconnectHandlerThe handler properly implements the
DisconnectHandlermethod to clean up event subscriptions, which is important to prevent memory leaks. This is a good practice when working with platform-specific handlers.MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs (1)
13-27: Handler properly implements ViewHandler for signature canvasThe implementation correctly inherits from
ViewHandler<FreakySignatureCanvasView, Canvas>and creates a Windows-specificCanvascontrol with appropriate background color. This is a good implementation of the MAUI handler pattern.
| private void OnTextChanged(AutoSuggestBox sender, AutoSuggestBoxTextChangedEventArgs args) | ||
| { | ||
| // Handle text changed | ||
| } | ||
|
|
||
| private void OnSuggestionChosen(AutoSuggestBox sender, AutoSuggestBoxSuggestionChosenEventArgs args) | ||
| { | ||
| // Handle suggestion chosen | ||
| } | ||
|
|
||
| private void OnQuerySubmitted(AutoSuggestBox sender, AutoSuggestBoxQuerySubmittedEventArgs args) | ||
| { | ||
| // Handle query submitted | ||
| } |
There was a problem hiding this comment.
Event handlers need implementation
The event handler methods contain only comments and don't actually implement any functionality. These need to be implemented to make the autocomplete feature work. You should forward these events to the virtual element (FreakyAutoCompleteView) or implement the autocomplete logic directly.
Here's a basic implementation example:
private void OnTextChanged(AutoSuggestBox sender, AutoSuggestBoxTextChangedEventArgs args)
{
- // Handle text changed
+ if (args.Reason == AutoSuggestionBoxTextChangeReason.UserInput)
+ {
+ // Update suggestions based on text
+ if (VirtualView != null)
+ {
+ VirtualView.Text = sender.Text;
+ // Trigger the TextChanged event or command on the virtual view
+ }
+ }
}
private void OnSuggestionChosen(AutoSuggestBox sender, AutoSuggestBoxSuggestionChosenEventArgs args)
{
- // Handle suggestion chosen
+ if (VirtualView != null && args.SelectedItem != null)
+ {
+ // Handle the selected suggestion
+ // VirtualView.SelectedItem = args.SelectedItem;
+ }
}
private void OnQuerySubmitted(AutoSuggestBox sender, AutoSuggestBoxQuerySubmittedEventArgs args)
{
- // Handle query submitted
+ if (VirtualView != null)
+ {
+ // Handle query submission
+ // VirtualView.QuerySubmitted(sender.Text, args.ChosenSuggestion);
+ }
}| public partial class FreakySignatureCanvasViewHandler : ViewHandler<FreakySignatureCanvasView, Canvas> | ||
| { | ||
| } No newline at end of file | ||
| #if WINDOWS | ||
| protected override Canvas CreatePlatformView() | ||
| { | ||
| var canvas = new Canvas | ||
| { | ||
| Background = new SolidColorBrush(Colors.White) | ||
| }; | ||
|
|
||
| canvas.PointerPressed += OnPointerPressed; | ||
| canvas.PointerMoved += OnPointerMoved; | ||
| canvas.PointerReleased += OnPointerReleased; | ||
| return canvas; | ||
| } | ||
|
|
||
| protected override void ConnectHandler(Canvas platformView) | ||
| { | ||
| base.ConnectHandler(platformView); | ||
| // Additional setup if needed | ||
| } | ||
|
|
||
| protected override void DisconnectHandler(Canvas platformView) | ||
| { | ||
| base.DisconnectHandler(platformView); | ||
| platformView.PointerPressed -= OnPointerPressed; | ||
| platformView.PointerMoved -= OnPointerMoved; | ||
| platformView.PointerReleased -= OnPointerReleased; | ||
| } | ||
|
|
||
| private bool _isDrawing; | ||
| private Windows.Foundation.Point _previousPoint; | ||
|
|
||
| private void OnPointerPressed(object sender, PointerRoutedEventArgs e) | ||
| { | ||
| if (sender is not Canvas canvas) | ||
| return; | ||
|
|
||
| _isDrawing = true; | ||
| _previousPoint = e.GetCurrentPoint(canvas).Position; | ||
| } | ||
|
|
||
| private void OnPointerMoved(object sender, PointerRoutedEventArgs e) | ||
| { | ||
| if (!_isDrawing || sender is not Canvas canvas) | ||
| return; | ||
|
|
||
| var currentPoint = e.GetCurrentPoint(canvas).Position; | ||
|
|
||
| var line = new Line | ||
| { | ||
| X1 = _previousPoint.X, | ||
| Y1 = _previousPoint.Y, | ||
| X2 = currentPoint.X, | ||
| Y2 = currentPoint.Y, | ||
| Stroke = new SolidColorBrush(Colors.Black), | ||
| StrokeThickness = 2 | ||
| }; | ||
|
|
||
| canvas.Children.Add(line); | ||
| _previousPoint = currentPoint; | ||
| } | ||
|
|
||
| private void OnPointerReleased(object sender, PointerRoutedEventArgs e) | ||
| { | ||
| _isDrawing = false; | ||
| } | ||
| #endif | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add support for clearing the signature
The handler doesn't implement any method to clear the signature canvas. You should add a method to handle the clearing of the signature when requested by the virtual view.
Add a method to clear the canvas:
#if WINDOWS
public void ClearSignature()
{
if (PlatformView != null)
{
PlatformView.Children.Clear();
}
}
#endifAnd add a mapper for handling property changes:
public static PropertyMapper<FreakySignatureCanvasView, FreakySignatureCanvasViewHandler> PropertyMapper = new(ViewHandler.ViewMapper)
{
[nameof(FreakySignatureCanvasView.IsClear)] = MapIsClear
};
public static void MapIsClear(FreakySignatureCanvasViewHandler handler, FreakySignatureCanvasView view)
{
if (view.IsClear)
{
handler.ClearSignature();
}
}| var line = new Line | ||
| { | ||
| X1 = _previousPoint.X, | ||
| Y1 = _previousPoint.Y, | ||
| X2 = currentPoint.X, | ||
| Y2 = currentPoint.Y, | ||
| Stroke = new SolidColorBrush(Colors.Black), | ||
| StrokeThickness = 2 | ||
| }; | ||
|
|
||
| canvas.Children.Add(line); | ||
| _previousPoint = currentPoint; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add customization from virtual view properties
The drawing implementation uses hardcoded values for stroke color and thickness. You should use properties from the virtual view to allow customization of the signature appearance.
var line = new Line
{
X1 = _previousPoint.X,
Y1 = _previousPoint.Y,
X2 = currentPoint.X,
Y2 = currentPoint.Y,
- Stroke = new SolidColorBrush(Colors.Black),
- StrokeThickness = 2
+ Stroke = new SolidColorBrush(VirtualView?.StrokeColor?.ToWindowsColor() ?? Colors.Black),
+ StrokeThickness = VirtualView?.StrokeThickness ?? 2
};📝 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.
| var line = new Line | |
| { | |
| X1 = _previousPoint.X, | |
| Y1 = _previousPoint.Y, | |
| X2 = currentPoint.X, | |
| Y2 = currentPoint.Y, | |
| Stroke = new SolidColorBrush(Colors.Black), | |
| StrokeThickness = 2 | |
| }; | |
| canvas.Children.Add(line); | |
| _previousPoint = currentPoint; | |
| } | |
| var line = new Line | |
| { | |
| X1 = _previousPoint.X, | |
| Y1 = _previousPoint.Y, | |
| X2 = currentPoint.X, | |
| Y2 = currentPoint.Y, | |
| Stroke = new SolidColorBrush(VirtualView?.StrokeColor?.ToWindowsColor() ?? Colors.Black), | |
| StrokeThickness = VirtualView?.StrokeThickness ?? 2 | |
| }; | |
| canvas.Children.Add(line); | |
| _previousPoint = currentPoint; | |
| } |
| canvas.PointerPressed += OnPointerPressed; | ||
| canvas.PointerMoved += OnPointerMoved; | ||
| canvas.PointerReleased += OnPointerReleased; | ||
| return canvas; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move event subscriptions to ConnectHandler
Event handlers are being attached in CreatePlatformView() but the common pattern is to attach them in ConnectHandler() for consistency with the MAUI handler lifecycle. In the current implementation, the events are attached in CreatePlatformView() but detached in DisconnectHandler().
protected override Canvas CreatePlatformView()
{
var canvas = new Canvas
{
Background = new SolidColorBrush(Colors.White)
};
- canvas.PointerPressed += OnPointerPressed;
- canvas.PointerMoved += OnPointerMoved;
- canvas.PointerReleased += OnPointerReleased;
return canvas;
}
protected override void ConnectHandler(Canvas platformView)
{
base.ConnectHandler(platformView);
- // Additional setup if needed
+ platformView.PointerPressed += OnPointerPressed;
+ platformView.PointerMoved += OnPointerMoved;
+ platformView.PointerReleased += OnPointerReleased;
}📝 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.
| canvas.PointerPressed += OnPointerPressed; | |
| canvas.PointerMoved += OnPointerMoved; | |
| canvas.PointerReleased += OnPointerReleased; | |
| return canvas; | |
| protected override Canvas CreatePlatformView() | |
| { | |
| var canvas = new Canvas | |
| { | |
| Background = new SolidColorBrush(Colors.White) | |
| }; | |
| return canvas; | |
| } | |
| protected override void ConnectHandler(Canvas platformView) | |
| { | |
| base.ConnectHandler(platformView); | |
| platformView.PointerPressed += OnPointerPressed; | |
| platformView.PointerMoved += OnPointerMoved; | |
| platformView.PointerReleased += OnPointerReleased; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs (3)
22-35: 🛠️ Refactor suggestionEvent handlers should be wired in ConnectHandler instead of CreatePlatformView.
The event handlers are being attached in
CreatePlatformView()but should be moved toConnectHandler()for consistency with the MAUI handler lifecycle pattern. This ensures proper separation between view creation and event wiring.protected override Canvas CreatePlatformView() { var canvas = new Canvas { Background = new SolidColorBrush(Colors.White) }; - canvas.PointerPressed += OnPointerPressed; - canvas.PointerMoved += OnPointerMoved; - canvas.PointerReleased += OnPointerReleased; return canvas; } protected override void ConnectHandler(Canvas platformView) { base.ConnectHandler(platformView); + platformView.PointerPressed += OnPointerPressed; + platformView.PointerMoved += OnPointerMoved; + platformView.PointerReleased += OnPointerReleased; }
69-77: 🛠️ Refactor suggestionUse stroke properties from the virtual view instead of hardcoded values.
The drawing implementation uses hardcoded values for stroke color and thickness. You should use the properties from the virtual view to allow customization of the signature appearance.
var line = new Line { X1 = _previousPoint.X, Y1 = _previousPoint.Y, X2 = currentPoint.X, Y2 = currentPoint.Y, - Stroke = new SolidColorBrush(Colors.Black), - StrokeThickness = 2 + Stroke = new SolidColorBrush(VirtualView?.StrokeColor?.ToWindowsColor() ?? Colors.Black), + StrokeThickness = VirtualView?.StrokeThickness ?? 2 };
50-60: 🛠️ Refactor suggestionMissing implementation for clearing the signature.
The handler doesn't implement any method to clear the signature canvas when requested by the virtual view. You need to add property mapping and a clear method.
Add these mappings and method:
// Add to the property mapper public static IPropertyMapper<FreakySignatureCanvasView, FreakySignatureCanvasViewHandler> Mapper = new PropertyMapper<FreakySignatureCanvasView, FreakySignatureCanvasViewHandler>(ViewHandler.ViewMapper) { [nameof(FreakySignatureCanvasView.IsClear)] = MapIsClear }; public static void MapIsClear(FreakySignatureCanvasViewHandler handler, FreakySignatureCanvasView view) { if (view.IsClear) { handler.ClearSignature(); } } // Add method to clear the signature public void ClearSignature() { if (PlatformView != null) { PlatformView.Children.Clear(); } }MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs (1)
45-58:⚠️ Potential issueEvent handlers need implementation.
The event handler methods contain only TODO comments and don't actually implement any functionality. These need to be implemented to make the autocomplete feature work.
Here's a basic implementation example:
private void OnTextChanged(AutoSuggestBox sender, AutoSuggestBoxTextChangedEventArgs args) { - // TODO: Handle text change + if (args.Reason == AutoSuggestionBoxTextChangeReason.UserInput && VirtualView != null) + { + VirtualView.Text = sender.Text; + VirtualView.TextChanged(); + } } private void OnSuggestionChosen(AutoSuggestBox sender, AutoSuggestBoxSuggestionChosenEventArgs args) { - // TODO: Handle suggestion chosen + if (VirtualView != null && args.SelectedItem != null) + { + VirtualView.SelectedItem = args.SelectedItem; + } } private void OnQuerySubmitted(AutoSuggestBox sender, AutoSuggestBoxQuerySubmittedEventArgs args) { - // TODO: Handle query submitted + if (VirtualView != null) + { + VirtualView.QuerySubmitted(sender.Text, args.ChosenSuggestion); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs(1 hunks)MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
🔇 Additional comments (3)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs (1)
13-20: Good implementation of the handler base class structure.The class now properly inherits from
ViewHandler<FreakySignatureCanvasView, Canvas>and sets up the property mapper. This structure aligns with the MAUI handler pattern and will allow the control to work correctly on Windows.MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.cs (2)
8-15: Good implementation of the handler base class structure.The class now properly inherits from
ViewHandler<FreakyAutoCompleteView, AutoSuggestBox>and sets up the property mapper. This structure aligns with the MAUI handler pattern and will allow the control to work correctly on Windows.
27-43: LGTM! Good implementation of the event handler lifecycle.The implementation correctly attaches event handlers in
ConnectHandlerand detaches them inDisconnectHandler, which follows the proper lifecycle pattern for MAUI handlers.
| #if WINDOWS | ||
| protected override AutoSuggestBox CreatePlatformView() | ||
| { | ||
| return new AutoSuggestBox | ||
| { | ||
| PlaceholderText = "Type here...", | ||
| IsSuggestionListOpen = false | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
PlaceholderText should be mapped from the virtual view property.
Currently, the placeholder text is hardcoded to "Type here...". Instead, you should use the placeholder text from the virtual view to ensure consistency with cross-platform settings.
protected override AutoSuggestBox CreatePlatformView()
{
return new AutoSuggestBox
{
- PlaceholderText = "Type here...",
+ PlaceholderText = VirtualView?.Placeholder ?? "Type here...",
IsSuggestionListOpen = false
};
}| public static IPropertyMapper<FreakyAutoCompleteView, FreakyAutoCompleteViewHandler> Mapper = | ||
| new PropertyMapper<FreakyAutoCompleteView, FreakyAutoCompleteViewHandler>(ViewHandler.ViewMapper); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add property mappings for common properties.
The Mapper doesn't contain any specific property mappings. You should add mappings for Text, Placeholder, and ItemsSource to sync the virtual view with the platform view.
public static IPropertyMapper<FreakyAutoCompleteView, FreakyAutoCompleteViewHandler> Mapper =
- new PropertyMapper<FreakyAutoCompleteView, FreakyAutoCompleteViewHandler>(ViewHandler.ViewMapper);
+ new PropertyMapper<FreakyAutoCompleteView, FreakyAutoCompleteViewHandler>(ViewHandler.ViewMapper)
+ {
+ [nameof(FreakyAutoCompleteView.Text)] = MapText,
+ [nameof(FreakyAutoCompleteView.Placeholder)] = MapPlaceholder,
+ [nameof(FreakyAutoCompleteView.ItemsSource)] = MapItemsSource
+ };
+ public static void MapText(FreakyAutoCompleteViewHandler handler, FreakyAutoCompleteView view)
+ {
+ handler.PlatformView.Text = view.Text;
+ }
+
+ public static void MapPlaceholder(FreakyAutoCompleteViewHandler handler, FreakyAutoCompleteView view)
+ {
+ handler.PlatformView.PlaceholderText = view.Placeholder;
+ }
+
+ public static void MapItemsSource(FreakyAutoCompleteViewHandler handler, FreakyAutoCompleteView view)
+ {
+ handler.PlatformView.ItemsSource = view.ItemsSource;
+ }📝 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.
| public static IPropertyMapper<FreakyAutoCompleteView, FreakyAutoCompleteViewHandler> Mapper = | |
| new PropertyMapper<FreakyAutoCompleteView, FreakyAutoCompleteViewHandler>(ViewHandler.ViewMapper); | |
| public static IPropertyMapper<FreakyAutoCompleteView, FreakyAutoCompleteViewHandler> Mapper = | |
| new PropertyMapper<FreakyAutoCompleteView, FreakyAutoCompleteViewHandler>(ViewHandler.ViewMapper) | |
| { | |
| [nameof(FreakyAutoCompleteView.Text)] = MapText, | |
| [nameof(FreakyAutoCompleteView.Placeholder)] = MapPlaceholder, | |
| [nameof(FreakyAutoCompleteView.ItemsSource)] = MapItemsSource | |
| }; | |
| public static void MapText(FreakyAutoCompleteViewHandler handler, FreakyAutoCompleteView view) | |
| { | |
| handler.PlatformView.Text = view.Text; | |
| } | |
| public static void MapPlaceholder(FreakyAutoCompleteViewHandler handler, FreakyAutoCompleteView view) | |
| { | |
| handler.PlatformView.PlaceholderText = view.Placeholder; | |
| } | |
| public static void MapItemsSource(FreakyAutoCompleteViewHandler handler, FreakyAutoCompleteView view) | |
| { | |
| handler.PlatformView.ItemsSource = view.ItemsSource; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs (3)
31-34: 🛠️ Refactor suggestionMove event subscriptions to ConnectHandler
Event handlers are being attached in
CreatePlatformView()but the common pattern is to attach them inConnectHandler()for consistency with the MAUI handler lifecycle. This ensures proper synchronization with the MAUI control lifecycle.protected override Canvas CreatePlatformView() { var canvas = new Canvas { Background = new SolidColorBrush(Colors.White) }; - canvas.PointerPressed += OnPointerPressed; - canvas.PointerMoved += OnPointerMoved; - canvas.PointerReleased += OnPointerReleased; return canvas; } protected override void ConnectHandler(Canvas platformView) { base.ConnectHandler(platformView); + platformView.PointerPressed += OnPointerPressed; + platformView.PointerMoved += OnPointerMoved; + platformView.PointerReleased += OnPointerReleased; }
76-77: 🛠️ Refactor suggestionUse virtual view properties for drawing customization
The drawing implementation uses hardcoded values for stroke color and thickness instead of properties from the virtual view.
var line = new Line { X1 = _previousPoint.X, Y1 = _previousPoint.Y, X2 = currentPoint.X, Y2 = currentPoint.Y, - Stroke = new SolidColorBrush(Colors.Black), - StrokeThickness = 2 + Stroke = new SolidColorBrush(VirtualView?.StrokeColor?.ToWindowsColor() ?? Colors.Black), + StrokeThickness = VirtualView?.StrokeThickness ?? 2 };
51-89: 🛠️ Refactor suggestionAdd support for clearing the signature
The handler doesn't implement any method to clear the signature canvas. You should add a method to handle the clearing when requested by the virtual view.
#if WINDOWS public void ClearSignature() { if (PlatformView != null) { PlatformView.Children.Clear(); } } #endif
🧹 Nitpick comments (1)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs (1)
64-82: Add bounds checking for drawing operationsThe current implementation doesn't check if drawing operations stay within the canvas bounds. This could lead to drawing outside the visible area or cause unexpected behavior.
private void OnPointerMoved(object sender, PointerRoutedEventArgs e) { if (!_isDrawing || sender is not Canvas canvas) return; var currentPoint = e.GetCurrentPoint(canvas).Position; + // Ensure points are within canvas bounds + currentPoint.X = Math.Clamp(currentPoint.X, 0, canvas.ActualWidth); + currentPoint.Y = Math.Clamp(currentPoint.Y, 0, canvas.ActualHeight); var line = new Line { X1 = _previousPoint.X, Y1 = _previousPoint.Y, X2 = currentPoint.X, Y2 = currentPoint.Y, Stroke = new SolidColorBrush(Colors.Black), StrokeThickness = 2 }; canvas.Children.Add(line); _previousPoint = currentPoint; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
| public partial class FreakySignatureCanvasViewHandler : ViewHandler<FreakySignatureCanvasView, Canvas> | ||
| { | ||
| public static IPropertyMapper<FreakySignatureCanvasView, FreakySignatureCanvasViewHandler> Mapper = | ||
| new PropertyMapper<FreakySignatureCanvasView, FreakySignatureCanvasViewHandler>(ViewHandler.ViewMapper); | ||
|
|
||
| public FreakySignatureCanvasViewHandler() : base(Mapper) | ||
| { | ||
| } | ||
|
|
||
| #if WINDOWS | ||
| protected override Canvas CreatePlatformView() | ||
| { | ||
| var canvas = new Canvas | ||
| { | ||
| Background = new SolidColorBrush(Colors.White) | ||
| }; | ||
|
|
||
| canvas.PointerPressed += OnPointerPressed; | ||
| canvas.PointerMoved += OnPointerMoved; | ||
| canvas.PointerReleased += OnPointerReleased; | ||
|
|
||
| return canvas; | ||
| } | ||
|
|
||
| protected override void ConnectHandler(Canvas platformView) | ||
| { | ||
| base.ConnectHandler(platformView); | ||
| } | ||
|
|
||
| protected override void DisconnectHandler(Canvas platformView) | ||
| { | ||
| base.DisconnectHandler(platformView); | ||
| platformView.PointerPressed -= OnPointerPressed; | ||
| platformView.PointerMoved -= OnPointerMoved; | ||
| platformView.PointerReleased -= OnPointerReleased; | ||
| } | ||
|
|
||
| private bool _isDrawing; | ||
| private Windows.Foundation.Point _previousPoint; | ||
|
|
||
| private void OnPointerPressed(object sender, PointerRoutedEventArgs e) | ||
| { | ||
| if (sender is not Canvas canvas) | ||
| return; | ||
|
|
||
| _isDrawing = true; | ||
| _previousPoint = e.GetCurrentPoint(canvas).Position; | ||
| } | ||
|
|
||
| private void OnPointerMoved(object sender, PointerRoutedEventArgs e) | ||
| { | ||
| if (!_isDrawing || sender is not Canvas canvas) | ||
| return; | ||
|
|
||
| var currentPoint = e.GetCurrentPoint(canvas).Position; | ||
|
|
||
| var line = new Line | ||
| { | ||
| X1 = _previousPoint.X, | ||
| Y1 = _previousPoint.Y, | ||
| X2 = currentPoint.X, | ||
| Y2 = currentPoint.Y, | ||
| Stroke = new SolidColorBrush(Colors.Black), | ||
| StrokeThickness = 2 | ||
| }; | ||
|
|
||
| canvas.Children.Add(line); | ||
| _previousPoint = currentPoint; | ||
| } | ||
|
|
||
| private void OnPointerReleased(object sender, PointerRoutedEventArgs e) | ||
| { | ||
| _isDrawing = false; | ||
| } | ||
| #endif | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add support for capturing the signature image
The handler doesn't implement functionality to capture the drawn signature as an image. This is typically a key feature for signature controls.
#if WINDOWS
public async Task<Stream> GetImageStreamAsync(ImageFormat format)
{
if (PlatformView == null)
return null;
// Render the canvas to a RenderTargetBitmap
var rtb = new RenderTargetBitmap();
await rtb.RenderAsync(PlatformView);
// Convert to stream in the requested format
var stream = new InMemoryRandomAccessStream();
var encoder = await BitmapEncoder.CreateAsync(
format == ImageFormat.Png ? BitmapEncoder.PngEncoderId : BitmapEncoder.JpegEncoderId,
stream);
var pixelBuffer = await rtb.GetPixelsAsync();
encoder.SetPixelData(
BitmapPixelFormat.Bgra8,
BitmapAlphaMode.Premultiplied,
(uint)rtb.PixelWidth,
(uint)rtb.PixelHeight,
96,
96,
pixelBuffer.ToArray());
await encoder.FlushAsync();
// Convert to MemoryStream for cross-platform compatibility
var ms = new MemoryStream();
var reader = new DataReader(stream.GetInputStreamAt(0));
await reader.LoadAsync((uint)stream.Size);
byte[] bytes = new byte[stream.Size];
reader.ReadBytes(bytes);
ms.Write(bytes, 0, bytes.Length);
ms.Position = 0;
return ms;
}
#endifWould you like me to add mapper code to support this functionality as well?
| public static IPropertyMapper<FreakySignatureCanvasView, FreakySignatureCanvasViewHandler> Mapper = | ||
| new PropertyMapper<FreakySignatureCanvasView, FreakySignatureCanvasViewHandler>(ViewHandler.ViewMapper); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement property mappings for signature customization
The property mapper is defined but doesn't map any properties specific to the signature view, such as stroke color, thickness, or clear functionality.
public static IPropertyMapper<FreakySignatureCanvasView, FreakySignatureCanvasViewHandler> Mapper =
- new PropertyMapper<FreakySignatureCanvasView, FreakySignatureCanvasViewHandler>(ViewHandler.ViewMapper);
+ new PropertyMapper<FreakySignatureCanvasView, FreakySignatureCanvasViewHandler>(ViewHandler.ViewMapper)
+ {
+ [nameof(FreakySignatureCanvasView.StrokeColor)] = MapStrokeColor,
+ [nameof(FreakySignatureCanvasView.StrokeThickness)] = MapStrokeThickness,
+ [nameof(FreakySignatureCanvasView.IsClear)] = MapIsClear
+ };
+ public static void MapStrokeColor(FreakySignatureCanvasViewHandler handler, FreakySignatureCanvasView view)
+ {
+ // Update stroke color if needed
+ }
+
+ public static void MapStrokeThickness(FreakySignatureCanvasViewHandler handler, FreakySignatureCanvasView view)
+ {
+ // Update stroke thickness if needed
+ }
+
+ public static void MapIsClear(FreakySignatureCanvasViewHandler handler, FreakySignatureCanvasView view)
+ {
+ if (view.IsClear)
+ {
+ handler.ClearSignature();
+ }
+ }
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MAUI.FreakyControls/MAUI.FreakyControls/Maui.FreakyControls.csproj(1 hunks)MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs
| <!-- Android --> | ||
| <ItemGroup Condition="$(TargetFramework.StartsWith('net9.0-android')) != true"> | ||
| <Compile Remove="**\*.Android.cs" /> | ||
| <None Include="**\*.Android.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | ||
| <Compile Remove="**\Android\**\*.cs" /> | ||
| <None Include="**\Android\**\*.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
Fix MSBuild Condition syntax for Android file exclusion.
The Condition attribute is using $(TargetFramework.StartsWith('net9.0-android')) != true, which isn’t a valid MSBuild property function. You need to invoke .NET string methods via the $([System.String]::…) syntax so the condition evaluates correctly per target framework.
Apply this diff to correct the ItemGroup header:
- <ItemGroup Condition="$(TargetFramework.StartsWith('net9.0-android')) != true">
+ <ItemGroup Condition="$([System.String]::StartsWith('$(TargetFramework)', 'net9.0-android')) == 'false'">| <!-- Windows --> | ||
| <ItemGroup Condition="$(TargetFramework.Contains('-windows')) != true"> | ||
| <Compile Remove="**\*.Windows.cs" /> | ||
| <None Include="**\*.Windows.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | ||
| <Compile Remove="**\Windows\**\*.cs" /> | ||
| <None Include="**\Windows\**\*.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
Fix MSBuild Condition syntax for Windows file exclusion.
The Windows block uses $(TargetFramework.Contains('-windows')), which isn’t valid in MSBuild. Use the property‐function syntax instead:
Suggested diff:
- <ItemGroup Condition="$(TargetFramework.Contains('-windows')) != true">
+ <ItemGroup Condition="$([System.String]::Contains('$(TargetFramework)', '-windows')) == 'false'">| <!-- iOS --> | ||
| <ItemGroup Condition="$(TargetFramework.StartsWith('net9.0-ios')) != true"> | ||
| <Compile Remove="**\*.iOS.cs" /> | ||
| <None Include="**\*.iOS.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | ||
| <Compile Remove="**\iOS\**\*.cs" /> | ||
| <None Include="**\iOS\**\*.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
Fix MSBuild Condition syntax for iOS file exclusion.
Similarly, the iOS block’s Condition uses $(TargetFramework.StartsWith('net9.0-ios')), which won’t be recognized. Switch to the MSBuild property‐function form:
Suggested change:
- <ItemGroup Condition="$(TargetFramework.StartsWith('net9.0-ios')) != true">
+ <ItemGroup Condition="$([System.String]::StartsWith('$(TargetFramework)', 'net9.0-ios')) == 'false'">📝 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.
| <!-- iOS --> | |
| <ItemGroup Condition="$(TargetFramework.StartsWith('net9.0-ios')) != true"> | |
| <Compile Remove="**\*.iOS.cs" /> | |
| <None Include="**\*.iOS.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | |
| <Compile Remove="**\iOS\**\*.cs" /> | |
| <None Include="**\iOS\**\*.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | |
| </ItemGroup> | |
| <!-- iOS --> | |
| <ItemGroup Condition="$([System.String]::StartsWith('$(TargetFramework)', 'net9.0-ios')) == 'false'"> | |
| <Compile Remove="**\*.iOS.cs" /> | |
| <None Include="**\*.iOS.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | |
| <Compile Remove="**\iOS\**\*.cs" /> | |
| <None Include="**\iOS\**\*.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | |
| </ItemGroup> |
| <!-- Mac Catalyst --> | ||
| <ItemGroup Condition="$(TargetFramework.StartsWith('net9.0-maccatalyst')) != true"> | ||
| <Compile Remove="**\*.MacCatalyst.cs" /> | ||
| <None Include="**\*.MacCatalyst.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | ||
| <Compile Remove="**\MacCatalyst\**\*.cs" /> | ||
| <None Include="**\MacCatalyst\**\*.cs" Exclude="$(DefaultItemExcludes);$(DefaultExcludesInProjectFolder)" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
Fix MSBuild Condition syntax for Mac Catalyst file exclusion.
The Mac Catalyst block also uses the unsupported $(TargetFramework.StartsWith('net9.0-maccatalyst')). Update it to call the .NET string method correctly:
Apply this diff:
- <ItemGroup Condition="$(TargetFramework.StartsWith('net9.0-maccatalyst')) != true">
+ <ItemGroup Condition="$([System.String]::StartsWith('$(TargetFramework)', 'net9.0-maccatalyst')) == 'false'">
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/NativeExtensions.cs (1)
32-45: Consider using consistent switch syntax style.While the implementation is correct, you're using different switch styles between this method and the previous one. For better code consistency, consider using the same syntax style in both methods.
- public static UIControlContentVerticalAlignment ToVPlatform(this Microsoft.Maui.TextAlignment alignment) - { - switch (alignment) - { - case Microsoft.Maui.TextAlignment.Start: - return UIControlContentVerticalAlignment.Top; - case Microsoft.Maui.TextAlignment.Center: - return UIControlContentVerticalAlignment.Center; - case Microsoft.Maui.TextAlignment.End: - return UIControlContentVerticalAlignment.Bottom; - default: - return UIControlContentVerticalAlignment.Center; // Default to center - } - } + public static UIControlContentVerticalAlignment ToVPlatform(this Microsoft.Maui.TextAlignment alignment) + { + return alignment switch + { + Microsoft.Maui.TextAlignment.Start => UIControlContentVerticalAlignment.Top, + Microsoft.Maui.TextAlignment.Center => UIControlContentVerticalAlignment.Center, + Microsoft.Maui.TextAlignment.End => UIControlContentVerticalAlignment.Bottom, + _ => UIControlContentVerticalAlignment.Center, // Default to center + }; + }MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/FreakyAutoCompleteViewHandler.ios.cs (1)
237-248: Defined but unused helper method.This helper method
UIFontDescriptorFromAttributesis defined but never used in the file. Either use it in the font handling logic or remove it if not needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
MAUI.FreakyControls/MAUI.FreakyControls/FreakyAutoCompleteView/IFreakyAutoCompleteView.cs(1 hunks)MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs(1 hunks)MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/FreakyAutoCompleteViewHandler.ios.cs(2 hunks)MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/NativeExtensions.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- MAUI.FreakyControls/MAUI.FreakyControls/Platforms/Windows/FreakySignatureViewHandler.win.cs
🧰 Additional context used
🪛 GitHub Check: CodeFactor
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/FreakyAutoCompleteViewHandler.ios.cs
[notice] 267-267: MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/FreakyAutoCompleteViewHandler.ios.cs#L267
A closing brace should not be preceded by a blank line. (SA1508)
[notice] 265-265: MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/FreakyAutoCompleteViewHandler.ios.cs#L265
An opening brace should not be followed by a blank line. (SA1505)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
🔇 Additional comments (5)
MAUI.FreakyControls/MAUI.FreakyControls/FreakyAutoCompleteView/IFreakyAutoCompleteView.cs (1)
18-23: Interface extension looks good!These new text formatting properties (alignment, font properties, and text transformation) will enable more granular control over the appearance of the auto-complete view. The property types are appropriate and consistent with MAUI conventions.
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/NativeExtensions.cs (1)
21-30: LGTM - Good implementation of TextAlignment extension method.The mapping from MAUI TextAlignment to UIKit.UITextAlignment is correctly implemented with appropriate default handling.
MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/FreakyAutoCompleteViewHandler.ios.cs (3)
26-28: Added initialization for new text formatting properties.You've correctly added the initial calls to update font, text alignment, and text transformation in the ConnectHandler method.
188-191: LGTM - HorizontalTextAlignment mapping looks good.The implementation correctly uses the extension method to convert MAUI TextAlignment to UIKit.UITextAlignment.
193-196: LGTM - VerticalTextAlignment mapping looks good.The implementation correctly uses the extension method to convert MAUI TextAlignment to UIControlContentVerticalAlignment.
| public static void MapFontFamily(FreakyAutoCompleteViewHandler handler, IFreakyAutoCompleteView view) | ||
| { | ||
| var font = UIFont.FromName(view.FontFamily, (float)view.FontSize); | ||
| handler.PlatformView.Font = font; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Font family setting may override font attributes.
This method sets the font based on fontFamily and fontSize but does not account for fontAttributes (bold/italic). If both properties are set, the behavior depends on which mapper runs last.
Consider a combined approach that respects all font properties:
- public static void MapFontFamily(FreakyAutoCompleteViewHandler handler, IFreakyAutoCompleteView view)
- {
- var font = UIFont.FromName(view.FontFamily, (float)view.FontSize);
- handler.PlatformView.Font = font;
- }
+ public static void MapFontFamily(FreakyAutoCompleteViewHandler handler, IFreakyAutoCompleteView view)
+ {
+ // If custom font family is specified
+ if (!string.IsNullOrEmpty(view.FontFamily))
+ {
+ UIFont font = UIFont.FromName(view.FontFamily, (float)view.FontSize);
+
+ // If font creation succeeded and attributes are specified
+ if (font != null && view.FontAttributes != FontAttributes.None)
+ {
+ // Try to create a font with the attributes
+ var descriptor = font.FontDescriptor.CreateWithTraits(UIFontDescriptorFromAttributes(view.FontAttributes));
+ UIFont attributedFont = UIFont.FromDescriptor(descriptor, (float)view.FontSize);
+
+ // Use the attributed font if available, otherwise fall back to the regular font
+ handler.PlatformView.Font = attributedFont ?? font;
+ }
+ else
+ {
+ handler.PlatformView.Font = font;
+ }
+ }
+ }Committable suggestion skipped: line range outside the PR's diff.
| private void UpdateTextTransform(FreakyNativeAutoCompleteView platformView) | ||
| { | ||
|
|
||
| } |
There was a problem hiding this comment.
Empty implementation with formatting issue.
The UpdateTextTransform method is called in ConnectHandler but has an empty implementation with a blank line that violates style rules.
Fix both the implementation and formatting:
private void UpdateTextTransform(FreakyNativeAutoCompleteView platformView)
{
-
+ if (VirtualView != null)
+ {
+ MapTextTransform(this, VirtualView);
+ }
}📝 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.
| private void UpdateTextTransform(FreakyNativeAutoCompleteView platformView) | |
| { | |
| } | |
| private void UpdateTextTransform(FreakyNativeAutoCompleteView platformView) | |
| { | |
| if (VirtualView != null) | |
| { | |
| MapTextTransform(this, VirtualView); | |
| } | |
| } |
🧰 Tools
🪛 GitHub Check: CodeFactor
[notice] 267-267: MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/FreakyAutoCompleteViewHandler.ios.cs#L267
A closing brace should not be preceded by a blank line. (SA1508)
[notice] 265-265: MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/FreakyAutoCompleteViewHandler.ios.cs#L265
An opening brace should not be followed by a blank line. (SA1505)
| public static void MapFontSize(FreakyAutoCompleteViewHandler handler, IFreakyAutoCompleteView view) | ||
| { | ||
| handler.PlatformView.Font = UIFont.SystemFontOfSize((float)view.FontSize); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
FontSize setter may override other font settings.
This method sets a system font with the specified size, but it doesn't preserve any font family or attributes that might have been set previously.
Consider this alternative approach:
- public static void MapFontSize(FreakyAutoCompleteViewHandler handler, IFreakyAutoCompleteView view)
- {
- handler.PlatformView.Font = UIFont.SystemFontOfSize((float)view.FontSize);
- }
+ public static void MapFontSize(FreakyAutoCompleteViewHandler handler, IFreakyAutoCompleteView view)
+ {
+ // Get current font
+ UIFont currentFont = handler.PlatformView.Font;
+
+ // If we have a current font, try to resize it
+ if (currentFont != null)
+ {
+ // Create a new font with the same family but new size
+ UIFont resizedFont = UIFont.FromName(currentFont.FamilyName, (float)view.FontSize);
+ if (resizedFont != null)
+ {
+ handler.PlatformView.Font = resizedFont;
+ return;
+ }
+ }
+
+ // Fallback to system font if no current font or resize failed
+ handler.PlatformView.Font = UIFont.SystemFontOfSize((float)view.FontSize);
+ }| private void UpdateTextAlignment(FreakyNativeAutoCompleteView platformView) | ||
| { | ||
| } |
There was a problem hiding this comment.
Empty implementation method.
The UpdateTextAlignment method is called in ConnectHandler but has an empty implementation. Implement it to properly initialize text alignment.
private void UpdateTextAlignment(FreakyNativeAutoCompleteView platformView)
{
+ if (VirtualView != null)
+ {
+ MapHorizontalTextAlignment(this, VirtualView);
+ MapVerticalTextAlignment(this, VirtualView);
+ }
}📝 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.
| private void UpdateTextAlignment(FreakyNativeAutoCompleteView platformView) | |
| { | |
| } | |
| private void UpdateTextAlignment(FreakyNativeAutoCompleteView platformView) | |
| { | |
| if (VirtualView != null) | |
| { | |
| MapHorizontalTextAlignment(this, VirtualView); | |
| MapVerticalTextAlignment(this, VirtualView); | |
| } | |
| } |
| private void UpdateFont(FreakyNativeAutoCompleteView platformView) | ||
| { | ||
| //platformView.font(VirtualView?.TextColor); | ||
| } |
There was a problem hiding this comment.
Empty implementation with commented code.
The UpdateFont method is called in ConnectHandler but has only a commented-out implementation. This method should either be implemented or removed if not needed.
If this method is intended to be used, implement it by calling the appropriate mappers:
private void UpdateFont(FreakyNativeAutoCompleteView platformView)
{
- //platformView.font(VirtualView?.TextColor);
+ if (VirtualView != null)
+ {
+ MapFontFamily(this, VirtualView);
+ MapFontAttributes(this, VirtualView);
+ MapFontSize(this, VirtualView);
+ }
}📝 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.
| private void UpdateFont(FreakyNativeAutoCompleteView platformView) | |
| { | |
| //platformView.font(VirtualView?.TextColor); | |
| } | |
| private void UpdateFont(FreakyNativeAutoCompleteView platformView) | |
| { | |
| if (VirtualView != null) | |
| { | |
| MapFontFamily(this, VirtualView); | |
| MapFontAttributes(this, VirtualView); | |
| MapFontSize(this, VirtualView); | |
| } | |
| } |
| public static void MapTextTransform(FreakyAutoCompleteViewHandler handler, IFreakyAutoCompleteView view) | ||
| { | ||
| if (view.TextTransform == TextTransform.Uppercase) | ||
| handler.PlatformView.Text = handler.PlatformView.Text.ToUpper(); | ||
| else if (view.TextTransform == TextTransform.Lowercase) | ||
| handler.PlatformView.Text = handler.PlatformView.Text.ToLower(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
TextTransform implementation might not handle text changes.
The current implementation directly modifies the text when the TextTransform property changes, but it doesn't account for subsequent text changes. This might lead to inconsistent behavior.
Consider implementing a transform method that is called whenever text changes:
public static void MapTextTransform(FreakyAutoCompleteViewHandler handler, IFreakyAutoCompleteView view)
{
- if (view.TextTransform == TextTransform.Uppercase)
- handler.PlatformView.Text = handler.PlatformView.Text.ToUpper();
- else if (view.TextTransform == TextTransform.Lowercase)
- handler.PlatformView.Text = handler.PlatformView.Text.ToLower();
+ // Store the transform setting in the platform view
+ handler.PlatformView.TextTransform = view.TextTransform;
+
+ // Apply transform to current text
+ if (!string.IsNullOrEmpty(handler.PlatformView.Text))
+ {
+ handler.PlatformView.Text = ApplyTextTransform(handler.PlatformView.Text, view.TextTransform);
+ }
}
+
+ // Helper to apply text transformation
+ private static string ApplyTextTransform(string text, TextTransform transform)
+ {
+ return transform switch
+ {
+ TextTransform.Uppercase => text.ToUpper(),
+ TextTransform.Lowercase => text.ToLower(),
+ _ => text,
+ };
+ }You would also need to modify the text change event handler to apply the transform to new text.
📝 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.
| public static void MapTextTransform(FreakyAutoCompleteViewHandler handler, IFreakyAutoCompleteView view) | |
| { | |
| if (view.TextTransform == TextTransform.Uppercase) | |
| handler.PlatformView.Text = handler.PlatformView.Text.ToUpper(); | |
| else if (view.TextTransform == TextTransform.Lowercase) | |
| handler.PlatformView.Text = handler.PlatformView.Text.ToLower(); | |
| } | |
| public static void MapTextTransform(FreakyAutoCompleteViewHandler handler, IFreakyAutoCompleteView view) | |
| { | |
| // Store the transform setting in the platform view | |
| handler.PlatformView.TextTransform = view.TextTransform; | |
| // Apply transform to current text | |
| if (!string.IsNullOrEmpty(handler.PlatformView.Text)) | |
| { | |
| handler.PlatformView.Text = ApplyTextTransform(handler.PlatformView.Text, view.TextTransform); | |
| } | |
| } | |
| // Helper to apply text transformation | |
| private static string ApplyTextTransform(string text, TextTransform transform) | |
| { | |
| return transform switch | |
| { | |
| TextTransform.Uppercase => text.ToUpper(), | |
| TextTransform.Lowercase => text.ToLower(), | |
| _ => text, | |
| }; | |
| } |
| public static void MapFontAttributes(FreakyAutoCompleteViewHandler handler, IFreakyAutoCompleteView view) | ||
| { | ||
| // If the FontAttributes is Bold | ||
| if (view.FontAttributes == FontAttributes.Bold) | ||
| { | ||
| handler.PlatformView.Font = UIFont.BoldSystemFontOfSize((nfloat)view.FontSize); | ||
| } | ||
| // If the FontAttributes is Italic | ||
| else if (view.FontAttributes == FontAttributes.Italic) | ||
| { | ||
| handler.PlatformView.Font = UIFont.ItalicSystemFontOfSize((nfloat)view.FontSize); | ||
| } | ||
| // If it's neither Bold nor Italic, set to normal font | ||
| else | ||
| { | ||
| handler.PlatformView.Font = UIFont.SystemFontOfSize((nfloat)view.FontSize); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Font attributes may be overridden by other font setting methods.
The MapFontAttributes method correctly sets font based on bold/italic attributes, but this might be overridden if MapFontFamily or MapFontSize is called afterward. Consider a more integrated approach to font handling.
Check if there's a designated order of property mapping methods in MAUI handlers or if you need to implement a combined font handling approach:
🏁 Script executed:
#!/bin/bash
# Look for patterns of font handling in other MAUI handlers
rg -A 5 -B 5 "FontAttributes|FontFamily|FontSize" --type csLength of output: 66607
🏁 Script executed:
#!/bin/bash
# Show the beginning of the iOS handler to inspect property mapper registration
sed -n '1,200p' MAUI.FreakyControls/MAUI.FreakyControls/Platforms/iOS/FreakyAutoCompleteViewHandler.ios.csLength of output: 8843
Combine font mapping to avoid one override another
Currently in Platforms/iOS/FreakyAutoCompleteViewHandler.ios.cs you have:
- MapFontAttributes (lines 198–215)
- MapFontFamily
- MapFontSize
Each of these independently does
handler.PlatformView.Font = …;so a later change to FontFamily or FontSize will wipe out any Bold/Italic traits you applied earlier (and vice versa).
• Refactor into a single MapFont method that reads all three properties at once—FontFamily, FontSize and FontAttributes—and uses a UIFontDescriptor to combine them. For example:
var family = view.FontFamily ?? UIFont.SystemFontOfSize(view.FontSize).Name;
var descriptor = UIFontDescriptor.FromName(family, (nfloat)view.FontSize)
.CreateWithTraits(UIFontDescriptorFromAttributes(view.FontAttributes));
handler.PlatformView.Font = UIFont.FromDescriptor(descriptor, (nfloat)view.FontSize);• Update your property mapper to call only this unified MapFont instead of three separate handlers.
This ensures you never lose the requested font style when any one of those properties changes.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
MAUI.FreakyControls/MAUI.FreakyControls/FreakyAutoCompleteView/FreakyAutoCompleteView.cs (1)
1-251: 💡 Verification agent🧩 Analysis chain
Verify that Windows platform handler is correctly set up to map these new properties
According to the PR title, this fixes a
HandlerNotFoundExceptionissue in Windows Machine mode. The addition of these text formatting properties should be connected to the Windows platform handler implementation to ensure it can properly map these properties.Let's verify that the Windows handler implementation exists and properly maps these new properties:
🏁 Script executed:
#!/bin/bash # Check the Windows platform handler implementation echo "Checking Windows platform handler implementation..." fd "FreakyAutoCompleteViewHandler" --type f --glob "*.cs" | xargs catLength of output: 274
🏁 Script executed:
#!/bin/bash echo "Locating Windows handler implementation for FreakyAutoCompleteView..." # Try locating the handler by filename files=$(fd -e cs "FreakyAutoCompleteViewHandler.cs") if [ -n "$files" ]; then echo "Found handler file(s) by name:" echo "$files" for f in $files; do echo -e "\n--- $f (first 200 lines) ---" sed -n '1,200p' "$f" done exit 0 fi # Fallback: search for the class declaration files=$(rg -l "class FreakyAutoCompleteViewHandler" -g '*.cs') if [ -n "$files" ]; then echo "Found handler file(s) by class pattern:" echo "$files" for f in $files; do echo -e "\n--- $f (first 200 lines) ---" sed -n '1,200p' "$f" done else echo "No Windows handler implementation found for FreakyAutoCompleteView." fiLength of output: 7955
Add missing property mappings in the Windows
FreakyAutoCompleteViewHandlerThe Windows handler currently only uses the default
ViewMapperand doesn’t map any of the new control properties. You’ll need to updateMAUI.FreakyControls/Platforms/Windows/FreakyAutoCompleteViewHandler.csto include entries in itsMapperfor all of the properties added inFreakyAutoCompleteView, for example:• Text → MapText
• TextColor → MapTextColor
• Placeholder → MapPlaceholder
• PlaceholderColor → MapPlaceholderColor
• TextMemberPath → MapTextMemberPath
• DisplayMemberPath → MapDisplayMemberPath
• IsSuggestionListOpen → MapIsSuggestionListOpen
• Threshold → MapThreshold
• UpdateTextOnSelect → MapUpdateTextOnSelect
• ItemsSource → MapItemsSource
• ImageAlignment/ImageSource/ImageCommand/ImageCommandParameter/ImageHeight/ImagePadding/ImageWidth → MapImageSource
• HorizontalTextAlignment → MapHorizontalTextAlignment
• VerticalTextAlignment → MapVerticalTextAlignment
• FontFamily → MapFontFamily
• FontSize → MapFontSize
• FontAttributes → MapFontAttributes
• TextTransform → MapTextTransformMake sure each
MapXXXmethod is implemented (or exists) to update theAutoSuggestBoxappropriately (either in the mapper itself or inConnectHandler).
🧹 Nitpick comments (2)
MAUI.FreakyControls/MAUI.FreakyControls/FreakyAutoCompleteView/FreakyAutoCompleteView.cs (2)
13-18: Add XML documentation for new propertiesAdding documentation comments would help developers understand the purpose and expected values for these new properties.
Consider adding XML documentation comments for each property:
+ /// <summary> + /// Gets or sets the horizontal text alignment. + /// </summary> public TextAlignment HorizontalTextAlignment { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
215-215: Remove trailing whitespaceThere's a line with only whitespace characters.
-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
MAUI.FreakyControls/MAUI.FreakyControls/FreakyAutoCompleteView/FreakyAutoCompleteView.cs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-plugin-ci
| public TextAlignment HorizontalTextAlignment { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } | ||
| public TextAlignment VerticalTextAlignment { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } | ||
| public string FontFamily { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } | ||
| public double FontSize { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } | ||
| public FontAttributes FontAttributes { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } | ||
| public TextTransform TextTransform { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } |
There was a problem hiding this comment.
Convert new properties to bindable properties to maintain consistency and enable XAML binding
These new properties are implemented with NotImplementedException which will cause runtime errors if used. More importantly, they don't follow the pattern used by other properties in this class - they should be implemented as bindable properties to enable proper XAML data binding.
Consider implementing these as bindable properties similar to the existing properties in this class. For example:
- public TextAlignment HorizontalTextAlignment { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
+ public TextAlignment HorizontalTextAlignment
+ {
+ get { return (TextAlignment)GetValue(HorizontalTextAlignmentProperty); }
+ set { SetValue(HorizontalTextAlignmentProperty, value); }
+ }
+
+ public static readonly BindableProperty HorizontalTextAlignmentProperty =
+ BindableProperty.Create(nameof(HorizontalTextAlignment), typeof(TextAlignment), typeof(FreakyAutoCompleteView), TextAlignment.Start, BindingMode.OneWay);Apply the same pattern for all six new properties to maintain consistency with the rest of the class.
💡 Verification agent
🧩 Analysis chain
These placeholder properties will cause runtime errors
All six new properties throw NotImplementedException, which will cause the application to crash if any of these properties are accessed at runtime.
🏁 Script executed:
#!/bin/bash
# Check if these properties are referenced elsewhere in the codebase
echo "Checking for references to the new text formatting properties..."
rg -A 1 "HorizontalTextAlignment|VerticalTextAlignment|FontFamily|FontSize|FontAttributes|TextTransform" --glob "*.cs" --glob "*.xaml" | grep -v "NotImplementedException" || echo "No direct usage found"Length of output: 74068
Implement proper BindableProperty-backed getters/setters for text‐formatting properties
The six new properties in FreakyAutoCompleteView.cs are currently throwing NotImplementedException, but they’re referenced throughout handlers and XAML (e.g. in MapHorizontalTextAlignment, MapFontSize, bindings in XAML). Any attempt to read or write them will crash at runtime.
Please replace the stubs at lines 13–18 of
MAUI.FreakyControls/MAUI.FreakyControls/FreakyAutoCompleteView/FreakyAutoCompleteView.cs
with BindableProperty definitions and getters/setters, for example:
-public TextAlignment HorizontalTextAlignment { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
+public static readonly BindableProperty HorizontalTextAlignmentProperty =
+ BindableProperty.Create(
+ nameof(HorizontalTextAlignment),
+ typeof(TextAlignment),
+ typeof(FreakyAutoCompleteView),
+ TextAlignment.Start);
+public TextAlignment HorizontalTextAlignment
+{
+ get => (TextAlignment)GetValue(HorizontalTextAlignmentProperty);
+ set => SetValue(HorizontalTextAlignmentProperty, value);
+}Repeat for VerticalTextAlignment, FontFamily, FontSize, FontAttributes and TextTransform, matching defaults and types from other controls. This ensures bindings and handler mappings won’t trigger a crash.
2 participants
Summary by CodeRabbit