Move default values into a style to support style overriding

[bijington]

Description of Change

I haven't fully tested this but from what I can see it certainly seems to solve the lack of Style support for the Popup properties. I suspect there may be a more efficient way to do this too, perhaps a single instance of the default style?

Linked Issues

• Fixes [BUG] Padding and Margin are not applied to the Popup class of Popup2 by Style attribute #2747

PR Checklist

• Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
• Has tests (if omitted, state reason in description)
• Has samples (if omitted, state reason in description)
• Rebased on top of main at time of PR
• Changes adhere to coding standard
• Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

Additional information

[Copilot]

Pull Request Overview

This PR moves the default property values for the Popup into a style so that they can be overridden, and adjusts the binding setups accordingly in both the Popup and PopupPage components.

• Default Popup property assignments now use a style added to the Popup’s Resources rather than inline assignments.
• Bindings in PopupPage.shared.cs have been updated to use statically imported property references.
• The sample App.xaml has been updated to demonstrate style overriding for Popup properties.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/CommunityToolkit.Maui/Views/Popup/PopupPage.shared.cs	Updated binding calls to use statically imported properties instead of explicit Popup properties.
src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs	Removed redundant BindableProperty declarations and moved default value assignments into a newly created style.
samples/CommunityToolkit.Maui.Sample/App.xaml	Added setters for Popup styling to support style overriding.

[bijington]

In fact I think this currently fails for ImplicitStylePopup

[TheCodeTraveler]

Thanks Shaun! I hadn't considered Style when working on Popup v2 and boy was that a glaring mistake.

Is this PR ready, or does it need more work for implicit styles like you mentioned in your comment above?

[bijington]

Thanks Shaun! I hadn't considered Style when working on Popup v2 and boy was that a glaring mistake.

Is this PR ready, or does it need more work for implicit styles like you mentioned in your comment above?

I can't decide if the behaviour is correct or not. Currently the ImplicitStylePopup will fill full screen because it doesn't define a value for HorizontalOptions or VerticalOptions. It does provide HeightRequest and WidthRequest which has no effect because of the first two that I mentioned.

I am not thinking that this behaviour is correct because this implicit style overrides our implicit style in the toolkit - there can only be one implicit style to use right?

[TheCodeTraveler]

I can't decide if the behaviour is correct or not. Currently the ImplicitStylePopup will fill full screen because it doesn't define a value for HorizontalOptions or VerticalOptions. It does provide HeightRequest and WidthRequest which has no effect because of the first two that I mentioned.

We don't want it to fill the screen by default; it should be centered by default.

I haven't had time to dig into the code - just a thought - this may be related to how we're propagating the HorizontalOptions and VerticalOptions via Bindings from the View that the user gives us onto the underlying Popup and underlying PopupBorder in PopupPage. CreatePopupFromView<T>() and PopupPage.PopupPageLayout():

PopupPage. CreatePopupFromView<T>()

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

Lines 127 to 148 in 28e030c

	protected static T CreatePopupFromView<T>(in View view) where T : Popup, new()
	{
	ArgumentNullException.ThrowIfNull(view);
	var popup = new T
	{
	BackgroundColor = view.BackgroundColor ??= PopupDefaults.BackgroundColor,
	Content = view
	};
	popup.SetBinding(BackgroundProperty, static (View view) => view.Background, source: view, mode: BindingMode.OneWay);
	popup.SetBinding(BindingContextProperty, static (View view) => view.BindingContext, source: view, mode: BindingMode.OneWay);
	popup.SetBinding(BackgroundColorProperty, static (View view) => view.BackgroundColor, source: view, mode: BindingMode.OneWay);
	popup.SetBinding(Popup.MarginProperty, static (View view) => view.Margin, source: view, mode: BindingMode.OneWay);
	popup.SetBinding(Popup.VerticalOptionsProperty, static (View view) => view.VerticalOptions, source: view, mode: BindingMode.OneWay, converter: new VerticalOptionsConverter());
	popup.SetBinding(Popup.HorizontalOptionsProperty, static (View view) => view.HorizontalOptions, source: view, mode: BindingMode.OneWay, converter: new HorizontalOptionsConverter());
	if (view is IPaddingElement paddingElement)
	{
	popup.SetBinding(Popup.PaddingProperty, static (IPaddingElement paddingElement) => paddingElement.Padding, source: paddingElement, mode: BindingMode.OneWay, converter: new PaddingConverter());
	}
	return popup;

PopupPage.PopupPageLayout()

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

Lines 186 to 220 in 28e030c

	public PopupPageLayout(in Popup popupContent, in IPopupOptions options, in ICommand tapOutsideOfPopupCommand)
	{
	Background = BackgroundColor = null;
	var tappableBackground = new BoxView
	{
	BackgroundColor = Colors.Transparent,
	HorizontalOptions = LayoutOptions.Fill,
	VerticalOptions = LayoutOptions.Fill
	};
	tappableBackground.GestureRecognizers.Add(new TapGestureRecognizer { Command = tapOutsideOfPopupCommand });
	Children.Add(tappableBackground); // Add the Tappable Background to the PopupPageLayout Grid before adding the Border to ensure the Border is displayed on top
	PopupBorder = new Border
	{
	BackgroundColor = popupContent.BackgroundColor ??= PopupDefaults.BackgroundColor,
	Content = popupContent
	};
	// Bind `Popup` values through to Border using OneWay Bindings
	PopupBorder.SetBinding(Border.MarginProperty, static (Popup popup) => popup.Margin, source: popupContent, mode: BindingMode.OneWay);
	PopupBorder.SetBinding(Border.PaddingProperty, static (Popup popup) => popup.Padding, source: popupContent, mode: BindingMode.OneWay);
	PopupBorder.SetBinding(Border.BackgroundProperty, static (Popup popup) => popup.Background, source: popupContent, mode: BindingMode.OneWay);
	PopupBorder.SetBinding(Border.BackgroundColorProperty, static (Popup popup) => popup.BackgroundColor, source: popupContent, mode: BindingMode.OneWay);
	PopupBorder.SetBinding(Border.VerticalOptionsProperty, static (Popup popup) => popup.VerticalOptions, source: popupContent, mode: BindingMode.OneWay);
	PopupBorder.SetBinding(Border.HorizontalOptionsProperty, static (Popup popup) => popup.HorizontalOptions, source: popupContent, mode: BindingMode.OneWay);
	// Bind `PopupOptions` values through to Border using OneWay Bindings
	PopupBorder.SetBinding(Border.ShadowProperty, static (IPopupOptions options) => options.Shadow, source: options, mode: BindingMode.OneWay);
	PopupBorder.SetBinding(Border.StrokeProperty, static (IPopupOptions options) => options.Shape, source: options, converter: new BorderStrokeConverter(), mode: BindingMode.OneWay);
	PopupBorder.SetBinding(Border.StrokeShapeProperty, static (IPopupOptions options) => options.Shape, source: options, mode: BindingMode.OneWay);
	PopupBorder.SetBinding(Border.StrokeThicknessProperty, static (IPopupOptions options) => options.Shape, source: options, converter: new BorderStrokeThicknessConverter(), mode: BindingMode.OneWay);
	Children.Add(PopupBorder);
	}

[mos379]

I really would like this change to be considered...