Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 20 additions & 57 deletions src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.ComponentModel;
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The using System.ComponentModel; directive doesn't appear to be used in this file. The [BindableProperty] attribute is defined in the CommunityToolkit.Maui namespace (auto-generated by the source generator), not System.ComponentModel.

If this directive is not used elsewhere in the file, consider removing it to keep imports clean.

Suggested change
using System.ComponentModel;

Copilot uses AI. Check for mistakes.
using CommunityToolkit.Maui.Extensions;

namespace CommunityToolkit.Maui.Views;
Expand All @@ -8,29 +9,38 @@
public partial class Popup : ContentView
{
/// <summary>
/// Bindable property to set the margin between the <see cref="Popup"/> and the edge of the window
/// Gets or sets the margin between the <see cref="Popup"/> and the edge of the window.
/// </summary>
public static new readonly BindableProperty MarginProperty = View.MarginProperty;
[BindableProperty(DefaultValueCreatorMethodName = nameof(CreateDefaultMargin))]
public partial Thickness Margin { get; set; }
static object CreateDefaultMargin(global::Microsoft.Maui.Controls.BindableObject? _) => Options.DefaultPopupSettings.Margin;

/// <summary>
/// Bindable property to set the padding between the <see cref="Popup"/> border and the <see cref="Popup"/> content
/// Gets or sets the padding between the <see cref="Popup"/> border and the <see cref="Popup"/> content.
/// </summary>
public static new readonly BindableProperty PaddingProperty = ContentView.PaddingProperty;
[BindableProperty(DefaultValueCreatorMethodName = nameof(CreateDefaultPadding))]
public partial Thickness Padding { get; set; }
static object CreateDefaultPadding(global::Microsoft.Maui.Controls.BindableObject? _) => Options.DefaultPopupSettings.Padding;

/// <summary>
/// Bindable property to set the horizontal position of the <see cref="Popup"/> when displayed on screen
/// Gets or sets the horizontal layout options used to position the <see cref="Popup"/> when displayed on screen.
/// </summary>
public static new readonly BindableProperty HorizontalOptionsProperty = View.HorizontalOptionsProperty;
[BindableProperty(DefaultValueCreatorMethodName = nameof(CreateDefaultHorizontalOptions))]
public partial LayoutOptions HorizontalOptions { get; set; }
static object CreateDefaultHorizontalOptions(global::Microsoft.Maui.Controls.BindableObject? _) => Options.DefaultPopupSettings.HorizontalOptions;

/// <summary>
/// Bindable property to set the vertical position of the <see cref="Popup"/> when displayed on screen
/// Gets or sets the vertical layout options used to position the <see cref="Popup"/> when displayed on screen.
/// </summary>
public static new readonly BindableProperty VerticalOptionsProperty = View.VerticalOptionsProperty;
[BindableProperty(DefaultValueCreatorMethodName = nameof(CreateDefaultVerticalOptions))]
public partial LayoutOptions VerticalOptions { get; set; }
static object CreateDefaultVerticalOptions(global::Microsoft.Maui.Controls.BindableObject? _) => Options.DefaultPopupSettings.VerticalOptions;

/// <summary>
/// Backing BindableProperty for the <see cref="CanBeDismissedByTappingOutsideOfPopup"/> property.
/// Gets or sets a value indicating whether the <see cref="Popup"/> can be dismissed by tapping outside of the popup.
/// </summary>
public static readonly BindableProperty CanBeDismissedByTappingOutsideOfPopupProperty = BindableProperty.Create(nameof(CanBeDismissedByTappingOutsideOfPopup), typeof(bool), typeof(Popup), Options.DefaultPopupSettings.CanBeDismissedByTappingOutsideOfPopup);
[BindableProperty(DefaultValue = PopupDefaults.CanBeDismissedByTappingOutsideOfPopup)]

Check failure on line 42 in src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs

View workflow job for this annotation

GitHub Actions / Build Sample App using Latest .NET SDK (macos-26)

The name 'PopupDefaults' does not exist in the current context

Check failure on line 42 in src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs

View workflow job for this annotation

GitHub Actions / Build Sample App using Latest .NET SDK (macos-26)

The name 'PopupDefaults' does not exist in the current context

Check failure on line 42 in src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs

View workflow job for this annotation

GitHub Actions / Build Sample App using Latest .NET SDK (macos-26)

The name 'PopupDefaults' does not exist in the current context

Check failure on line 42 in src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs

View workflow job for this annotation

GitHub Actions / Build Sample App using Latest .NET SDK (macos-26)

The name 'PopupDefaults' does not exist in the current context

Check failure on line 42 in src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs

View workflow job for this annotation

GitHub Actions / Build Sample App using Latest .NET SDK (macos-26)

The name 'PopupDefaults' does not exist in the current context

Check failure on line 42 in src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs

View workflow job for this annotation

GitHub Actions / Build Sample App using Latest .NET SDK (macos-26)

The name 'PopupDefaults' does not exist in the current context
public partial bool CanBeDismissedByTappingOutsideOfPopup { get; set; }

/// <summary>
/// Initializes Popup
Expand All @@ -54,53 +64,6 @@
/// </summary>
public event EventHandler? Closed;

/// <summary>
/// Sets the margin between the <see cref="Popup"/> and the edge of the window
/// </summary>
public new Thickness Margin
{
get => base.Margin;
set => base.Margin = value;
}

/// <summary>
/// Sets the padding between the <see cref="Popup"/> border and the <see cref="Popup"/> content
/// </summary>
public new Thickness Padding
{
get => base.Padding;
set => base.Padding = value;
}

/// <summary>
/// Sets the horizontal position of the <see cref="Popup"/> when displayed on screen
/// </summary>
public new LayoutOptions HorizontalOptions
{
get => base.HorizontalOptions;
set => base.HorizontalOptions = value;
}

/// <summary>
/// Sets the vertical position of the <see cref="Popup"/> when displayed on screen
/// </summary>
public new LayoutOptions VerticalOptions
{
get => base.VerticalOptions;
set => base.VerticalOptions = value;
}

/// <inheritdoc cref="IPopupOptions.CanBeDismissedByTappingOutsideOfPopup"/> />
/// <remarks>
/// When true and the user taps outside the popup, it will dismiss.
/// On Android - when false the hardware back button is disabled.
/// </remarks>
public bool CanBeDismissedByTappingOutsideOfPopup
{
get => (bool)GetValue(CanBeDismissedByTappingOutsideOfPopupProperty);
set => SetValue(CanBeDismissedByTappingOutsideOfPopupProperty, value);
}

/// <summary>
/// Close the Popup.
/// </summary>
Expand Down
74 changes: 25 additions & 49 deletions src/CommunityToolkit.Maui/Views/Popup/PopupOptions.shared.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,71 +8,47 @@ namespace CommunityToolkit.Maui;
public partial class PopupOptions : BindableObject, IPopupOptions
{
/// <summary>
/// Backing BindableProperty for the <see cref="CanBeDismissedByTappingOutsideOfPopup"/> property.
/// Gets or sets a value indicating whether the popup can be dismissed by tapping outside of the popup.
/// Default is provided by <see cref="Options.DefaultPopupOptionsSettings"/>.
/// </summary>
public static readonly BindableProperty CanBeDismissedByTappingOutsideOfPopupProperty = BindableProperty.Create(nameof(CanBeDismissedByTappingOutsideOfPopup), typeof(bool), typeof(PopupOptions), Options.DefaultPopupOptionsSettings.CanBeDismissedByTappingOutsideOfPopup);
[BindableProperty(DefaultValueCreatorMethodName = nameof(CreateCanBeDismissedByTappingOutsideOfPopup))]
public partial bool CanBeDismissedByTappingOutsideOfPopup { get; set; }
static object CreateCanBeDismissedByTappingOutsideOfPopup(BindableObject? _) => Options.DefaultPopupOptionsSettings.CanBeDismissedByTappingOutsideOfPopup;

/// <summary>
/// Backing BindableProperty for the <see cref="OnTappingOutsideOfPopup"/> property.
/// Gets or sets an <see cref="Action"/> invoked when the user taps outside of the popup.
/// Default is provided by <see cref="Options.DefaultPopupOptionsSettings"/>.
/// </summary>
public static readonly BindableProperty OnTappingOutsideOfPopupProperty = BindableProperty.Create(nameof(OnTappingOutsideOfPopup), typeof(Action), typeof(PopupOptions), Options.DefaultPopupOptionsSettings.OnTappingOutsideOfPopup);
[BindableProperty(DefaultValueCreatorMethodName = nameof(CreateOnTappingOutsideOfPopup))]
public partial Action? OnTappingOutsideOfPopup { get; set; }
static object? CreateOnTappingOutsideOfPopup(BindableObject? _) => Options.DefaultPopupOptionsSettings.OnTappingOutsideOfPopup;
Comment on lines +23 to +24
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nullability mismatch: The property is declared as non-nullable Action, but the default value creator method returns object? (nullable). This creates an inconsistency where the property signature suggests it's always non-null, but the default value can be null.

Consider either:

  1. Making the property nullable: public partial Action? OnTappingOutsideOfPopup { get; set; }
  2. Providing a non-null default value if the property should never be null

Note: The interface IPopupOptions.OnTappingOutsideOfPopup is nullable (Action?), so option 1 is recommended for consistency.

Copilot uses AI. Check for mistakes.

/// <summary>
/// Backing BindableProperty for the <see cref="PageOverlayColor"/> property.
/// Gets or sets the overlay <see cref="Color"/> applied to the page while the popup is displayed.
/// Default is provided by <see cref="Options.DefaultPopupOptionsSettings"/>.
/// </summary>
public static readonly BindableProperty PageOverlayColorProperty = BindableProperty.Create(nameof(PageOverlayColor), typeof(Color), typeof(PopupOptions), Options.DefaultPopupOptionsSettings.PageOverlayColor);
[BindableProperty(DefaultValueCreatorMethodName = nameof(CreatePageOverlayColor))]
public partial Color PageOverlayColor { get; set; }
static object CreatePageOverlayColor(BindableObject? _) => Options.DefaultPopupOptionsSettings.PageOverlayColor;
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Nullability mismatch: The property is declared as non-nullable Color, but the default value creator method can potentially return a nullable value if Options.DefaultPopupOptionsSettings.PageOverlayColor is null. While Color is a struct and typically non-nullable, this could cause issues if the default settings are not properly initialized.

Verify that Options.DefaultPopupOptionsSettings.PageOverlayColor is guaranteed to be non-null, or update the default value creator to handle potential null cases explicitly.

Suggested change
static object CreatePageOverlayColor(BindableObject? _) => Options.DefaultPopupOptionsSettings.PageOverlayColor;
static object CreatePageOverlayColor(BindableObject? _)
{
return Options.DefaultPopupOptionsSettings.PageOverlayColor is not null
? Options.DefaultPopupOptionsSettings.PageOverlayColor
: Colors.Transparent;
}

Copilot uses AI. Check for mistakes.

/// <summary>
/// Backing BindableProperty for the <see cref="Shape"/> property.
/// Gets or sets the <see cref="Shape"/> used to render the popup's outline.
/// Default is provided by <see cref="Options.DefaultPopupOptionsSettings"/>.
/// </summary>
public static readonly BindableProperty ShapeProperty = BindableProperty.Create(nameof(Shape), typeof(Shape), typeof(PopupOptions), Options.DefaultPopupOptionsSettings.Shape);
[BindableProperty(DefaultValueCreatorMethodName = nameof(CreateShape))]
public partial Shape? Shape { get; set; }
static object? CreateShape(BindableObject? _) => Options.DefaultPopupOptionsSettings.Shape;

/// <summary>
/// Backing BindableProperty for the <see cref="Shadow"/> property.
/// Gets or sets the <see cref="Shadow"/> applied to the popup.
/// Default is provided by <see cref="Options.DefaultPopupOptionsSettings"/>.
/// </summary>
public static readonly BindableProperty ShadowProperty = BindableProperty.Create(nameof(Shadow), typeof(Shadow), typeof(PopupOptions), Options.DefaultPopupOptionsSettings.Shadow);
[BindableProperty(DefaultValueCreatorMethodName = nameof(CreateShadow))]
public partial Shadow? Shadow { get; set; }
static object? CreateShadow(BindableObject? _) => Options.DefaultPopupOptionsSettings.Shadow;

/// <summary>
/// An empty instance of <see cref="IPopupOptions"/> containing default values.
/// </summary>
public static IPopupOptions Empty { get; } = new PopupOptions();

/// <inheritdoc/>
/// <remarks>
/// When true and the user taps outside the popup, it will dismiss.
/// On Android - when false the hardware back button is disabled.
/// </remarks>
public bool CanBeDismissedByTappingOutsideOfPopup
{
get => (bool)GetValue(CanBeDismissedByTappingOutsideOfPopupProperty);
set => SetValue(CanBeDismissedByTappingOutsideOfPopupProperty, value);
}

/// <inheritdoc/>
public Color PageOverlayColor
{
get => (Color)GetValue(PageOverlayColorProperty);
set => SetValue(PageOverlayColorProperty, value);
}

/// <inheritdoc/>
public Action? OnTappingOutsideOfPopup
{
get => (Action?)GetValue(OnTappingOutsideOfPopupProperty);
set => SetValue(OnTappingOutsideOfPopupProperty, value);
}

/// <inheritdoc/>
public Shape? Shape
{
get => (Shape?)GetValue(ShapeProperty);
set => SetValue(ShapeProperty, value);
}

/// <inheritdoc/>
public Shadow? Shadow
{
get => (Shadow?)GetValue(ShadowProperty);
set => SetValue(ShadowProperty, value);
}
}
Loading