Skip to content

Commit a4b3339

Browse files
ne0rrmatrixCopilotTheCodeTraveler
authored
Bindable property for Popup (#2991)
* Refactor Popup properties to use BindableProperty attribute Replaced manual BindableProperty implementations with the BindableProperty attribute in the `Popup` and `PopupOptions` classes. This simplifies property definitions, reduces boilerplate, and improves code readability. Updated properties such as `Margin`, `Padding`, `HorizontalOptions`, and `VerticalOptions` in `Popup`, and `CanBeDismissedByTappingOutsideOfPopup`, `OnTappingOutsideOfPopup`, `PageOverlayColor`, `Shape`, and `Shadow` in `PopupOptions` to use the attribute with default value creators. Removed redundant manual property implementations as they are now handled by the BindableProperty attribute. * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/CommunityToolkit.Maui/Views/Popup/PopupOptions.shared.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Refactor Popup property default value handling Updated the `CanBeDismissedByTappingOutsideOfPopup` property to use a `DefaultValueCreatorMethodName` for dynamic default value resolution. Added the `CreateCanBeDismissedByTappingOutsideOfPopup` method to provide the default value at runtime, improving flexibility and reducing reliance on hardcoded defaults. * Use partial property initializers * Fix XML Comments * Fix Failing Unit Tests * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Brandon Minnick <13558917+TheCodeTraveler@users.noreply.github.com>
1 parent 960d0b5 commit a4b3339

File tree

4 files changed

+31
-109
lines changed

4 files changed

+31
-109
lines changed

src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ public void ShowPopupAsync_WithCustomOptions_AppliesOptions()
445445
var popupPage = (PopupPage)navigation.ModalStack[0];
446446
var popupPageContent = popupPage.Content;
447447
var border = popupPageContent.PopupBorder;
448-
var popup = border.Content;
448+
var popup = (Popup)(border.Content ?? throw new InvalidOperationException("Content cannot be null"));
449449

450450
// Assert
451451
Assert.NotNull(popup);
@@ -516,7 +516,7 @@ public void ShowPopupAsync_Shell_WithCustomOptions_AppliesOptions()
516516
var popupPage = (PopupPage)shellNavigation.ModalStack[0];
517517
var popupPageContent = popupPage.Content;
518518
var border = popupPageContent.PopupBorder;
519-
var popup = border.Content;
519+
var popup = (ShortLivedSelfClosingPopup)(border.Content ?? throw new InvalidOperationException("Content cannot be null"));
520520

521521
// Assert
522522
Assert.NotNull(popup);

src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ public void ShowPopupAsync_WithCustomOptions_AppliesOptions()
165165
var popupPage = (PopupPage)navigation.ModalStack[0];
166166
var popupPageLayout = popupPage.Content;
167167
var border = popupPageLayout.PopupBorder;
168-
var popup = border.Content;
168+
var popup = (MockPopup)(border.Content ?? throw new InvalidOperationException("Content cannot be null"));
169169

170170
// Assert
171171
Assert.NotNull(popup);
@@ -515,7 +515,6 @@ public async Task ClosePopupAsyncT_ShouldClosePopupUsingPageAndReturnResult()
515515
}
516516
}
517517

518-
519518
class GarbageCollectionHeavySelfClosingPopup(MockPageViewModel viewModel, object? result = null) : MockSelfClosingPopup(viewModel, TimeSpan.FromMilliseconds(500), result)
520519
{
521520
protected override void HandlePopupOpened(object? sender, EventArgs e)

src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs

Lines changed: 17 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
12
using CommunityToolkit.Maui.Extensions;
23

34
namespace CommunityToolkit.Maui.Views;
@@ -7,31 +8,6 @@ namespace CommunityToolkit.Maui.Views;
78
/// </summary>
89
public partial class Popup : ContentView
910
{
10-
/// <summary>
11-
/// Bindable property to set the margin between the <see cref="Popup"/> and the edge of the window
12-
/// </summary>
13-
public static new readonly BindableProperty MarginProperty = View.MarginProperty;
14-
15-
/// <summary>
16-
/// Bindable property to set the padding between the <see cref="Popup"/> border and the <see cref="Popup"/> content
17-
/// </summary>
18-
public static new readonly BindableProperty PaddingProperty = ContentView.PaddingProperty;
19-
20-
/// <summary>
21-
/// Bindable property to set the horizontal position of the <see cref="Popup"/> when displayed on screen
22-
/// </summary>
23-
public static new readonly BindableProperty HorizontalOptionsProperty = View.HorizontalOptionsProperty;
24-
25-
/// <summary>
26-
/// Bindable property to set the vertical position of the <see cref="Popup"/> when displayed on screen
27-
/// </summary>
28-
public static new readonly BindableProperty VerticalOptionsProperty = View.VerticalOptionsProperty;
29-
30-
/// <summary>
31-
/// Backing BindableProperty for the <see cref="CanBeDismissedByTappingOutsideOfPopup"/> property.
32-
/// </summary>
33-
public static readonly BindableProperty CanBeDismissedByTappingOutsideOfPopupProperty = BindableProperty.Create(nameof(CanBeDismissedByTappingOutsideOfPopup), typeof(bool), typeof(Popup), Options.DefaultPopupSettings.CanBeDismissedByTappingOutsideOfPopup);
34-
3511
/// <summary>
3612
/// Initializes Popup
3713
/// </summary>
@@ -55,51 +31,36 @@ public Popup()
5531
public event EventHandler? Closed;
5632

5733
/// <summary>
58-
/// Sets the margin between the <see cref="Popup"/> and the edge of the window
34+
/// Gets or sets the margin between the <see cref="Popup"/> and the edge of the window.
5935
/// </summary>
60-
public new Thickness Margin
61-
{
62-
get => base.Margin;
63-
set => base.Margin = value;
64-
}
36+
[BindableProperty]
37+
public new partial Thickness Margin { get; set; } = Options.DefaultPopupSettings.Margin;
6538

6639
/// <summary>
67-
/// Sets the padding between the <see cref="Popup"/> border and the <see cref="Popup"/> content
40+
/// Gets or sets the padding between the <see cref="Popup"/> border and the <see cref="Popup"/> content.
6841
/// </summary>
69-
public new Thickness Padding
70-
{
71-
get => base.Padding;
72-
set => base.Padding = value;
73-
}
42+
[BindableProperty]
43+
public new partial Thickness Padding { get; set; } = Options.DefaultPopupSettings.Padding;
7444

7545
/// <summary>
76-
/// Sets the horizontal position of the <see cref="Popup"/> when displayed on screen
46+
/// Gets or sets the horizontal position of the <see cref="Popup"/> when displayed on screen.
7747
/// </summary>
78-
public new LayoutOptions HorizontalOptions
79-
{
80-
get => base.HorizontalOptions;
81-
set => base.HorizontalOptions = value;
82-
}
48+
[BindableProperty]
49+
public new partial LayoutOptions HorizontalOptions { get; set; } = Options.DefaultPopupSettings.HorizontalOptions;
8350

8451
/// <summary>
85-
/// Sets the vertical position of the <see cref="Popup"/> when displayed on screen
52+
/// Gets or sets the vertical position of the <see cref="Popup"/> when displayed on screen.
8653
/// </summary>
87-
public new LayoutOptions VerticalOptions
88-
{
89-
get => base.VerticalOptions;
90-
set => base.VerticalOptions = value;
91-
}
54+
[BindableProperty]
55+
public new partial LayoutOptions VerticalOptions { get; set; } = Options.DefaultPopupSettings.VerticalOptions;
9256

9357
/// <inheritdoc cref="IPopupOptions.CanBeDismissedByTappingOutsideOfPopup"/> />
9458
/// <remarks>
9559
/// When true and the user taps outside the popup, it will dismiss.
9660
/// On Android - when false the hardware back button is disabled.
9761
/// </remarks>
98-
public bool CanBeDismissedByTappingOutsideOfPopup
99-
{
100-
get => (bool)GetValue(CanBeDismissedByTappingOutsideOfPopupProperty);
101-
set => SetValue(CanBeDismissedByTappingOutsideOfPopupProperty, value);
102-
}
62+
[BindableProperty]
63+
public partial bool CanBeDismissedByTappingOutsideOfPopup { get; set; } = Options.DefaultPopupSettings.CanBeDismissedByTappingOutsideOfPopup;
10364

10465
/// <summary>
10566
/// Close the Popup.
@@ -148,5 +109,7 @@ public partial class Popup<T> : Popup
148109
}
149110

150111
sealed class PopupNotFoundException() : InvalidPopupOperationException($"Unable to close popup: could not locate {nameof(PopupPage)}. {nameof(PopupExtensions.ShowPopup)} or {nameof(PopupExtensions.ShowPopupAsync)} must be called before {nameof(Popup.CloseAsync)}. If using a custom implementation of {nameof(Popup)}, override the {nameof(Popup.CloseAsync)} method");
112+
151113
sealed class PopupBlockedException(in Page currentVisibleModalPage) : InvalidPopupOperationException($"Unable to close Popup because it is blocked by the Modal Page {currentVisibleModalPage.GetType().FullName}. Please call `{nameof(Page.Navigation)}.{nameof(Page.Navigation.PopModalAsync)}()` to first remove {currentVisibleModalPage.GetType().FullName} from the {nameof(Page.Navigation.ModalStack)}");
114+
152115
class InvalidPopupOperationException(in string message) : InvalidOperationException(message);

src/CommunityToolkit.Maui/Views/Popup/PopupOptions.shared.cs

Lines changed: 11 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -7,31 +7,6 @@ namespace CommunityToolkit.Maui;
77
/// </summary>
88
public partial class PopupOptions : BindableObject, IPopupOptions
99
{
10-
/// <summary>
11-
/// Backing BindableProperty for the <see cref="CanBeDismissedByTappingOutsideOfPopup"/> property.
12-
/// </summary>
13-
public static readonly BindableProperty CanBeDismissedByTappingOutsideOfPopupProperty = BindableProperty.Create(nameof(CanBeDismissedByTappingOutsideOfPopup), typeof(bool), typeof(PopupOptions), Options.DefaultPopupOptionsSettings.CanBeDismissedByTappingOutsideOfPopup);
14-
15-
/// <summary>
16-
/// Backing BindableProperty for the <see cref="OnTappingOutsideOfPopup"/> property.
17-
/// </summary>
18-
public static readonly BindableProperty OnTappingOutsideOfPopupProperty = BindableProperty.Create(nameof(OnTappingOutsideOfPopup), typeof(Action), typeof(PopupOptions), Options.DefaultPopupOptionsSettings.OnTappingOutsideOfPopup);
19-
20-
/// <summary>
21-
/// Backing BindableProperty for the <see cref="PageOverlayColor"/> property.
22-
/// </summary>
23-
public static readonly BindableProperty PageOverlayColorProperty = BindableProperty.Create(nameof(PageOverlayColor), typeof(Color), typeof(PopupOptions), Options.DefaultPopupOptionsSettings.PageOverlayColor);
24-
25-
/// <summary>
26-
/// Backing BindableProperty for the <see cref="Shape"/> property.
27-
/// </summary>
28-
public static readonly BindableProperty ShapeProperty = BindableProperty.Create(nameof(Shape), typeof(Shape), typeof(PopupOptions), Options.DefaultPopupOptionsSettings.Shape);
29-
30-
/// <summary>
31-
/// Backing BindableProperty for the <see cref="Shadow"/> property.
32-
/// </summary>
33-
public static readonly BindableProperty ShadowProperty = BindableProperty.Create(nameof(Shadow), typeof(Shadow), typeof(PopupOptions), Options.DefaultPopupOptionsSettings.Shadow);
34-
3510
/// <summary>
3611
/// An empty instance of <see cref="IPopupOptions"/> containing default values.
3712
/// </summary>
@@ -42,37 +17,22 @@ public partial class PopupOptions : BindableObject, IPopupOptions
4217
/// When true and the user taps outside the popup, it will dismiss.
4318
/// On Android - when false the hardware back button is disabled.
4419
/// </remarks>
45-
public bool CanBeDismissedByTappingOutsideOfPopup
46-
{
47-
get => (bool)GetValue(CanBeDismissedByTappingOutsideOfPopupProperty);
48-
set => SetValue(CanBeDismissedByTappingOutsideOfPopupProperty, value);
49-
}
20+
[BindableProperty]
21+
public partial bool CanBeDismissedByTappingOutsideOfPopup { get; set; } = Options.DefaultPopupOptionsSettings.CanBeDismissedByTappingOutsideOfPopup;
5022

5123
/// <inheritdoc/>
52-
public Color PageOverlayColor
53-
{
54-
get => (Color)GetValue(PageOverlayColorProperty);
55-
set => SetValue(PageOverlayColorProperty, value);
56-
}
24+
[BindableProperty]
25+
public partial Action? OnTappingOutsideOfPopup { get; set; } = Options.DefaultPopupOptionsSettings.OnTappingOutsideOfPopup;
5726

5827
/// <inheritdoc/>
59-
public Action? OnTappingOutsideOfPopup
60-
{
61-
get => (Action?)GetValue(OnTappingOutsideOfPopupProperty);
62-
set => SetValue(OnTappingOutsideOfPopupProperty, value);
63-
}
28+
[BindableProperty]
29+
public partial Color PageOverlayColor { get; set; } = Options.DefaultPopupOptionsSettings.PageOverlayColor;
6430

6531
/// <inheritdoc/>
66-
public Shape? Shape
67-
{
68-
get => (Shape?)GetValue(ShapeProperty);
69-
set => SetValue(ShapeProperty, value);
70-
}
32+
[BindableProperty]
33+
public partial Shape? Shape { get; set; } = Options.DefaultPopupOptionsSettings.Shape;
7134

72-
/// <inheritdoc/>
73-
public Shadow? Shadow
74-
{
75-
get => (Shadow?)GetValue(ShadowProperty);
76-
set => SetValue(ShadowProperty, value);
77-
}
35+
/// <inheritdoc cref="IPopupOptions.Shadow"/>
36+
[BindableProperty]
37+
public partial Shadow? Shadow { get; set; } = Options.DefaultPopupOptionsSettings.Shadow;
7838
}

0 commit comments

Comments
 (0)