Skip to content

Commit 6133ad1

Browse files
bijingtonTheCodeTraveler
bijington
and
TheCodeTraveler
authored
Prevent a tap inside the content from closing a popup (CommunityToolkit#2741)
* Prevent a tap inside the content from closing a popup * Revert "Prevent a tap inside the content from closing a popup" This reverts commit 173356b. * Make use of a BoxView as a background handler of taps * Add GestureRecognizer to Prevent Popup from closing when tapped inside Refactored PopupPageLayout to expose the Border via a property instead of accessing it through the Children collection. Updated PopupPage to add gesture recognizers directly and simplified content initialization. Adjusted related unit tests to use the new Border property for improved clarity and maintainability. * Fix NuGet Security Vulnerability * Revert "Add GestureRecognizer to Prevent Popup from closing when tapped inside" This reverts commit d17876e. * Fix unit tests * Remove the coalescing assignment that won't be invoked * Add `in`, Rename to `tappableBackground `, Move Border Initialization after `tappableBackground ` code * Add `public Border PopupBorder { get; }` and simplify unit tests --------- Co-authored-by: Shaun Lawrence <17139988+bijington@users.noreply.github.com> Co-authored-by: Brandon Minnick <13558917+TheCodeTraveler@users.noreply.github.com> Co-authored-by: Brandon Minnick <13558917+brminnick@users.noreply.github.com>
1 parent 2217d08 commit 6133ad1

File tree

5 files changed

+65
-39
lines changed

5 files changed

+65
-39
lines changed

‎src/CommunityToolkit.Maui.UnitTests/CommunityToolkit.Maui.UnitTests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
<ItemGroup>
2222
<!--Fix vulnerabilities-->
2323
<PackageReference Include="System.Net.Http" Version="4.3.4" />
24+
<PackageReference Include="System.Private.Uri" Version="4.3.2" />
2425
<PackageReference Include="System.Text.RegularExpressions" Version="4.3.1" />
2526
</ItemGroup>
2627

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ public void ShowPopupAsync_WithViewType_SetsCorrectDefaults()
192192
navigation.ShowPopup(label);
193193

194194
popupPage = (PopupPage)navigation.ModalStack[0];
195-
autogeneratedPopup = (Popup)(((Border)popupPage.Content.Children[0]).Content ?? throw new InvalidOperationException("Border Content cannot be null"));
195+
autogeneratedPopup = (Popup)(popupPage.Content.PopupBorder.Content ?? throw new InvalidOperationException("Border Content cannot be null"));
196196

197197
// Assert
198198
Assert.Equal(PopupDefaults.BackgroundColor, autogeneratedPopup.BackgroundColor);
@@ -435,7 +435,7 @@ public void ShowPopupAsync_WithCustomOptions_AppliesOptions()
435435

436436
var popupPage = (PopupPage)navigation.ModalStack[0];
437437
var popupPageContent = popupPage.Content;
438-
var border = (Border)popupPageContent.Children[0];
438+
var border = popupPageContent.PopupBorder;
439439
var popup = border.Content;
440440

441441
// Assert
@@ -507,7 +507,7 @@ public void ShowPopupAsync_Shell_WithCustomOptions_AppliesOptions()
507507

508508
var popupPage = (PopupPage)shellNavigation.ModalStack[0];
509509
var popupPageContent = popupPage.Content;
510-
var border = (Border)popupPageContent.Children[0];
510+
var border = popupPageContent.PopupBorder;
511511
var popup = border.Content;
512512

513513
// Assert
@@ -579,7 +579,7 @@ public void ShowPopupAsyncWithView_WithCustomOptions_AppliesOptions()
579579

580580
var popupPage = (PopupPage)navigation.ModalStack[0];
581581
var popupPageContent = popupPage.Content;
582-
var border = (Border)popupPageContent.Children[0];
582+
var border = popupPageContent.PopupBorder;
583583
var popup = (Popup)(border.Content ?? throw new InvalidCastException());
584584

585585
// Assert
@@ -660,7 +660,7 @@ public void ShowPopupAsyncWithView_Shell_WithCustomOptions_AppliesOptions()
660660

661661
var popupPage = (PopupPage)shellNavigation.ModalStack[0];
662662
var popupPageContent = popupPage.Content;
663-
var border = (Border)popupPageContent.Children[0];
663+
var border = popupPageContent.PopupBorder;
664664
var popup = (Popup)(border.Content ?? throw new InvalidCastException());
665665

666666
// Assert
@@ -1149,7 +1149,7 @@ public async Task ShowPopupAsync_ReferenceTypeShouldReturnNull_WhenPopupTapGestu
11491149
var popupPage = (PopupPage)navigation.ModalStack[0];
11501150
popupPage.PopupClosed += HandlePopupClosed;
11511151

1152-
var tapGestureRecognizer = (TapGestureRecognizer)popupPage.Content.GestureRecognizers[0];
1152+
var tapGestureRecognizer = GetTapOutsideGestureRecognizer(popupPage);
11531153
tapGestureRecognizer.Command?.Execute(null);
11541154

11551155
var popupClosedResult = await popupClosedTCS.Task;
@@ -1184,7 +1184,7 @@ public async Task ShowPopupAsync_Shell_ReferenceTypeShouldReturnNull_WhenPopupTa
11841184
var popupPage = (PopupPage)shellNavigation.ModalStack[0];
11851185
popupPage.PopupClosed += HandlePopupClosed;
11861186

1187-
var tapGestureRecognizer = (TapGestureRecognizer)popupPage.Content.GestureRecognizers[0];
1187+
var tapGestureRecognizer = GetTapOutsideGestureRecognizer(popupPage);
11881188
tapGestureRecognizer.Command?.Execute(null);
11891189

11901190
var popupClosedResult = await popupClosedTCS.Task;
@@ -1212,7 +1212,7 @@ public async Task ShowPopupAsync_NullableValueTypeShouldReturnResult_WhenPopupIs
12121212
var popupPage = (PopupPage)navigation.ModalStack[0];
12131213
popupPage.PopupClosed += HandlePopupClosed;
12141214

1215-
var tapGestureRecognizer = (TapGestureRecognizer)popupPage.Content.GestureRecognizers[0];
1215+
var tapGestureRecognizer = GetTapOutsideGestureRecognizer(popupPage);
12161216
tapGestureRecognizer.Command?.Execute(null);
12171217

12181218
var popupClosedResult = await popupClosedTCS.Task;
@@ -1247,7 +1247,7 @@ public async Task ShowPopupAsync_Shell_NullableValueTypeShouldReturnResult_WhenP
12471247
var popupPage = (PopupPage)shellNavigation.ModalStack[0];
12481248
popupPage.PopupClosed += HandlePopupClosed;
12491249

1250-
var tapGestureRecognizer = (TapGestureRecognizer)popupPage.Content.GestureRecognizers[0];
1250+
var tapGestureRecognizer = GetTapOutsideGestureRecognizer(popupPage);
12511251
tapGestureRecognizer.Command?.Execute(null);
12521252

12531253
var popupClosedResult = await popupClosedTCS.Task;
@@ -1275,7 +1275,7 @@ public async Task ShowPopupAsync_ValueTypeShouldReturnResult_WhenPopupIsClosedBy
12751275
var popupPage = (PopupPage)navigation.ModalStack[0];
12761276
popupPage.PopupClosed += HandlePopupClosed;
12771277

1278-
var tapGestureRecognizer = (TapGestureRecognizer)popupPage.Content.GestureRecognizers[0];
1278+
var tapGestureRecognizer = GetTapOutsideGestureRecognizer(popupPage);
12791279
tapGestureRecognizer.Command?.Execute(null);
12801280

12811281
var popupClosedResult = await popupClosedTCS.Task;
@@ -1311,7 +1311,7 @@ public async Task ShowPopupAsync_Shell_ValueTypeShouldReturnResult_WhenPopupIsCl
13111311
var popupPage = (PopupPage)shellNavigation.ModalStack[0];
13121312
popupPage.PopupClosed += HandlePopupClosed;
13131313

1314-
var tapGestureRecognizer = (TapGestureRecognizer)popupPage.Content.GestureRecognizers[0];
1314+
var tapGestureRecognizer = GetTapOutsideGestureRecognizer(popupPage);
13151315
tapGestureRecognizer.Command?.Execute(null);
13161316

13171317
var popupClosedResult = await popupClosedTCS.Task;
@@ -1459,6 +1459,9 @@ public async Task ShowPopupAsync_TaskShouldCompleteWhenCloseAsyncIsCalled()
14591459
Assert.Equal(expectedResult, popupResult.Result);
14601460
Assert.False(popupResult.WasDismissedByTappingOutsideOfPopup);
14611461
}
1462+
1463+
static TapGestureRecognizer GetTapOutsideGestureRecognizer(PopupPage popupPage) =>
1464+
(TapGestureRecognizer)popupPage.Content.Children.OfType<BoxView>().Single().GestureRecognizers[0];
14621465
}
14631466

14641467
sealed class ViewWithIQueryAttributable : Button, IQueryAttributable

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ public void ShowPopupAsync_WithCustomOptions_AppliesOptions()
166166

167167
var popupPage = (PopupPage)navigation.ModalStack[0];
168168
var popupPageLayout = popupPage.Content;
169-
var border = (Border)popupPageLayout.Children[0];
169+
var border = popupPageLayout.PopupBorder;
170170
var popup = border.Content;
171171

172172
// Assert

‎src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupPageTests.cs‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -250,13 +250,22 @@ public void Constructor_WithViewAndPopupOptions_SetsCorrectProperties()
250250
Assert.Equal(UIModalPresentationStyle.OverFullScreen, popupPage.On<iOS>().ModalPresentationStyle());
251251

252252
// Verify content has tap gesture recognizer attached
253-
Assert.Single(popupPage.Content.GestureRecognizers);
254-
Assert.IsType<TapGestureRecognizer>(popupPage.Content.GestureRecognizers[0]);
253+
var gestureRecognizers = popupPage.Content.Children.OfType<BoxView>().Single().GestureRecognizers;
254+
Assert.Single(gestureRecognizers);
255+
Assert.IsType<TapGestureRecognizer>(gestureRecognizers[0]);
255256

256257
// Verify PopupPageLayout structure
257258
var pageContent = popupPage.Content;
258-
Assert.Single(pageContent.Children);
259-
Assert.IsType<Border>(pageContent.Children[0]);
259+
Assert.Collection(
260+
pageContent.Children,
261+
first =>
262+
{
263+
first.Should().BeOfType<BoxView>();
264+
},
265+
second =>
266+
{
267+
second.Should().BeOfType<Border>();
268+
});
260269

261270
// Verify content binding context is set correctly
262271
Assert.Equal(view.BindingContext, pageContent.BindingContext);
@@ -316,7 +325,7 @@ public async Task TapGestureRecognizer_ShouldClosePopupWhenCanBeDismissedIsTrue(
316325

317326
var popupPage = new PopupPage(view, popupOptions);
318327

319-
var tapGestureRecognizer = (TapGestureRecognizer)popupPage.Content.GestureRecognizers[0];
328+
var tapGestureRecognizer = GetTapOutsideGestureRecognizer(popupPage);
320329
var command = tapGestureRecognizer.Command;
321330
Assert.NotNull(command);
322331

@@ -341,7 +350,7 @@ public void TapGestureRecognizer_ShouldNotExecuteWhenCanBeDismissedIsFalse()
341350
};
342351

343352
var popupPage = new PopupPage(view, popupOptions);
344-
var tapGestureRecognizer = (TapGestureRecognizer)popupPage.Content.GestureRecognizers[0];
353+
var tapGestureRecognizer = GetTapOutsideGestureRecognizer(popupPage);
345354
var command = tapGestureRecognizer.Command;
346355

347356
// Act & Assert
@@ -421,7 +430,7 @@ public void TappingOutside_ShouldNotClosePopup_WhenCanBeDismissedIsFalse()
421430
var popupPage = new PopupPage(view, popupOptions);
422431

423432
// Act
424-
var tapGestureRecognizer = (TapGestureRecognizer)popupPage.Content.GestureRecognizers[0];
433+
var tapGestureRecognizer = GetTapOutsideGestureRecognizer(popupPage);
425434
var command = tapGestureRecognizer.Command;
426435

427436
// Assert
@@ -472,12 +481,15 @@ public void PopupPage_ShouldRespectLayoutOptions()
472481

473482
// Act
474483
var popupPage = new PopupPage(view, PopupOptions.Empty);
475-
var border = (Border)popupPage.Content.Children[0];
484+
var border = popupPage.Content.PopupBorder;
476485

477486
// Assert
478487
Assert.Equal(LayoutOptions.Start, border.VerticalOptions);
479488
Assert.Equal(LayoutOptions.End, border.HorizontalOptions);
480489
}
490+
491+
static TapGestureRecognizer GetTapOutsideGestureRecognizer(PopupPage popupPage) =>
492+
(TapGestureRecognizer)popupPage.Content.Children.OfType<BoxView>().Single().GestureRecognizers[0];
481493

482494
// Helper class for testing protected methods
483495
sealed class TestablePopupPage(View view, IPopupOptions popupOptions) : PopupPage(view, popupOptions)

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

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.ComponentModel;
22
using System.Globalization;
3+
using System.Windows.Input;
34
using CommunityToolkit.Maui.Converters;
45
using CommunityToolkit.Maui.Core;
56
using CommunityToolkit.Maui.Extensions;
@@ -41,16 +42,14 @@ public PopupPage(Popup popup, IPopupOptions popupOptions)
4142
this.popup = popup;
4243
this.popupOptions = popupOptions;
4344

44-
// Only set the content if the parent constructor hasn't set the content already; don't override content if it already exists
45-
base.Content ??= new PopupPageLayout(popup, popupOptions);
46-
4745
tapOutsideOfPopupCommand = new Command(async () =>
4846
{
4947
popupOptions.OnTappingOutsideOfPopup?.Invoke();
5048
await CloseAsync(new PopupResult(true));
5149
}, () => popupOptions.CanBeDismissedByTappingOutsideOfPopup);
5250

53-
Content.GestureRecognizers.Add(new TapGestureRecognizer { Command = tapOutsideOfPopupCommand });
51+
// Only set the content if the parent constructor hasn't set the content already; don't override content if it already exists
52+
base.Content = new PopupPageLayout(popup, popupOptions, tapOutsideOfPopupCommand);
5453

5554
if (popupOptions is BindableObject bindablePopupOptions)
5655
{
@@ -104,15 +103,15 @@ public async Task CloseAsync(PopupResult result, CancellationToken token = defau
104103

105104
popupClosedEventManager.HandleEvent(this, result, nameof(PopupClosed));
106105
}
107-
106+
108107
protected override bool OnBackButtonPressed()
109108
{
110109
// Only close the Popup if PopupOptions.CanBeDismissedByTappingOutsideOfPopup is true
111110
if (popupOptions.CanBeDismissedByTappingOutsideOfPopup)
112111
{
113112
CloseAsync(new PopupResult(true), CancellationToken.None).SafeFireAndForget();
114113
}
115-
114+
116115
// Always return true to let the Android Operating System know that we are manually handling the Navigation request from the Android Back Button
117116
return true;
118117
}
@@ -176,33 +175,44 @@ void IQueryAttributable.ApplyQueryAttributes(IDictionary<string, object> query)
176175

177176
internal sealed partial class PopupPageLayout : Grid
178177
{
179-
public PopupPageLayout(in Popup popupContent, in IPopupOptions options)
178+
public PopupPageLayout(in Popup popupContent, in IPopupOptions options, in ICommand tapOutsideOfPopupCommand)
180179
{
181180
Background = BackgroundColor = null;
182181

183-
var border = new Border
182+
var tappableBackground = new BoxView
183+
{
184+
BackgroundColor = Colors.Transparent,
185+
HorizontalOptions = LayoutOptions.Fill,
186+
VerticalOptions = LayoutOptions.Fill
187+
};
188+
tappableBackground.GestureRecognizers.Add(new TapGestureRecognizer { Command = tapOutsideOfPopupCommand });
189+
Children.Add(tappableBackground); // Add the Tappable Background to the PopupPageLayout Grid before adding the Border to ensure the Border is displayed on top
190+
191+
PopupBorder = new Border
184192
{
185193
BackgroundColor = popupContent.BackgroundColor ??= PopupDefaults.BackgroundColor,
186194
Content = popupContent
187195
};
188196

189197
// Bind `Popup` values through to Border using OneWay Bindings
190-
border.SetBinding(Border.MarginProperty, static (Popup popup) => popup.Margin, source: popupContent, mode: BindingMode.OneWay);
191-
border.SetBinding(Border.PaddingProperty, static (Popup popup) => popup.Padding, source: popupContent, mode: BindingMode.OneWay);
192-
border.SetBinding(Border.BackgroundProperty, static (Popup popup) => popup.Background, source: popupContent, mode: BindingMode.OneWay);
193-
border.SetBinding(Border.BackgroundColorProperty, static (Popup popup) => popup.BackgroundColor, source: popupContent, mode: BindingMode.OneWay);
194-
border.SetBinding(Border.VerticalOptionsProperty, static (Popup popup) => popup.VerticalOptions, source: popupContent, mode: BindingMode.OneWay);
195-
border.SetBinding(Border.HorizontalOptionsProperty, static (Popup popup) => popup.HorizontalOptions, source: popupContent, mode: BindingMode.OneWay);
198+
PopupBorder.SetBinding(Border.MarginProperty, static (Popup popup) => popup.Margin, source: popupContent, mode: BindingMode.OneWay);
199+
PopupBorder.SetBinding(Border.PaddingProperty, static (Popup popup) => popup.Padding, source: popupContent, mode: BindingMode.OneWay);
200+
PopupBorder.SetBinding(Border.BackgroundProperty, static (Popup popup) => popup.Background, source: popupContent, mode: BindingMode.OneWay);
201+
PopupBorder.SetBinding(Border.BackgroundColorProperty, static (Popup popup) => popup.BackgroundColor, source: popupContent, mode: BindingMode.OneWay);
202+
PopupBorder.SetBinding(Border.VerticalOptionsProperty, static (Popup popup) => popup.VerticalOptions, source: popupContent, mode: BindingMode.OneWay);
203+
PopupBorder.SetBinding(Border.HorizontalOptionsProperty, static (Popup popup) => popup.HorizontalOptions, source: popupContent, mode: BindingMode.OneWay);
196204

197205
// Bind `PopupOptions` values through to Border using OneWay Bindings
198-
border.SetBinding(Border.ShadowProperty, static (IPopupOptions options) => options.Shadow, source: options, mode: BindingMode.OneWay);
199-
border.SetBinding(Border.StrokeProperty, static (IPopupOptions options) => options.Shape, source: options, converter: new BorderStrokeConverter(), mode: BindingMode.OneWay);
200-
border.SetBinding(Border.StrokeShapeProperty, static (IPopupOptions options) => options.Shape, source: options, mode: BindingMode.OneWay);
201-
border.SetBinding(Border.StrokeThicknessProperty, static (IPopupOptions options) => options.Shape, source: options, converter: new BorderStrokeThicknessConverter(), mode: BindingMode.OneWay);
206+
PopupBorder.SetBinding(Border.ShadowProperty, static (IPopupOptions options) => options.Shadow, source: options, mode: BindingMode.OneWay);
207+
PopupBorder.SetBinding(Border.StrokeProperty, static (IPopupOptions options) => options.Shape, source: options, converter: new BorderStrokeConverter(), mode: BindingMode.OneWay);
208+
PopupBorder.SetBinding(Border.StrokeShapeProperty, static (IPopupOptions options) => options.Shape, source: options, mode: BindingMode.OneWay);
209+
PopupBorder.SetBinding(Border.StrokeThicknessProperty, static (IPopupOptions options) => options.Shape, source: options, converter: new BorderStrokeThicknessConverter(), mode: BindingMode.OneWay);
202210

203-
Children.Add(border);
211+
Children.Add(PopupBorder);
204212
}
205213

214+
public Border PopupBorder { get; }
215+
206216
sealed partial class BorderStrokeThicknessConverter : BaseConverterOneWay<Shape?, double>
207217
{
208218
public override double DefaultConvertReturnValue { get; set; } = PopupOptionsDefaults.BorderStrokeThickness;

0 commit comments

Comments
 (0)