[Core] Implement BackgroundImageSource in PageHandler #3396
Conversation
jsuarezruiz
added
the
legacy-area-controls
src/Core/src/Core/IPage.cs
Outdated
| { /// <summary> | ||
| /// Represents a .NET MAUI Page. | ||
| /// </summary> | ||
| public interface IPage : IContentView |
There was a problem hiding this comment.
I don't think we want to create an IPage for this. Can we follow the same route as we do for things like ITitledElement? I feel like the interface for the background image needs to be more interesting then just having the IImageSource as well. Perhaps we use something like
interface IViewBackgroundImagePart : IImageSourcePart because then that makes it more compatible with other ImageLoading structures
You can then just instantiate a
public ImageSourcePartLoader SourceLoader =>
_imageSourcePartLoader ??= new ImageSourcePartLoader(this, () => VirtualView, OnSetImageSource);And use that to handle all the loading behavior.
At that point the mapper could possibly even just be on ViewHandler and thus a generalized BackgroundImage for native views is born
There was a problem hiding this comment.
Removed IPage and implemented IViewBackgroundImagePart. Also added a ContentPage Gallery sample where easily test this scenario.
There was a problem hiding this comment.
@PureWeen Could you review the PR again?. Thnx!
| { | ||
| } | ||
|
|
||
| public static void MapBackgroundImageSource(IPageHandler handler, IContentView page) |
There was a problem hiding this comment.
Can we move all of this up into ViewHandler?
The extension methods all seem to work against UIView/Android.View/Panel
WinUI is the only one that has kind of a specific type but could probably just use pattern matching to look through the types that accept a background.
This way anyone could annotate any view with IViewBackgroundImagePart and then in controls we will only implement IViewBackgroundImagePart at the page level
|
I think we have the potential to make a big improvement it IView supports a background image. Not sure how this will work with IImage having both an image and a background image. This is not really possible with interfaces because it will be the same interface type. SHould IImage support both a background image and an image. I am thinking it is 2 separate concepts, like a centered image that is animating, nd then a background gradient? This is probably the same with IImageButton. Maybe background should be a brush/paint actually? In fact, this is already a thing. I am wondering if Page.BackgroundImage should translate into |
|
I have an idea to actually use ImageBrush and Background instead of exposing new properties. Maybe under the hood we split it up since maui graphics doesn't use image sources. So we can do what we do with Background/BackgroundColor and basically have this: IImageSource IImageSourcePart.Source
{
get
{
if (Background is ImageBrush imageBrush && !imageBrush.IsEmpty)
return imageBrush.ImageSource;
return BackgroundImageSource;
}
}namespace Microsoft.Maui.Controls
{
[System.ComponentModel.TypeConverter(typeof(BrushTypeConverter))]
public class ImageBrush : Brush
{
public ImageBrush()
{
}
public ImageBrush(ImageSource imageSource)
{
ImageSource = imageSource;
}
public override bool IsEmpty =>
ImageSource?.IsEmpty ?? true;
public static readonly BindableProperty ImageSourceProperty = BindableProperty.Create(
nameof(ImageSource), typeof(ImageSource), typeof(ImageBrush), default(ImageSource));
public virtual ImageSource ImageSource
{
get => (ImageSource)GetValue(ImageSourceProperty);
set => SetValue(ImageSourceProperty, value);
}
public override bool Equals(object obj) =>
obj is ImageBrush dest && ImageSource == dest.ImageSource;
public override int GetHashCode() =>
-1234567890 + ImageSource.GetHashCode();
}
} |
mattleibow
dismissed
PureWeen’s stale review
Things have happened and changes were made!
Enhances #3396 and fixes the issue where windows did not support initial backgrounds anymore
Eilon
removed
the
legacy-area-controls
9 participants
Description of Change
Implement BackgroundImageSource in PageHandler.
PR Checklist
Does this PR touch anything that might affect accessibility?