Add support for a container view#1025
Conversation
| { | ||
| new AView? NativeView { get; } | ||
|
|
||
| new AView? ContainerView { get; } |
There was a problem hiding this comment.
Not sure if this should be on the interface.
| Actions = | ||
| { | ||
| [nameof(IFrameworkElement.InvalidateMeasure)] = MapInvalidateMeasure, | ||
| [nameof(ViewHandler.NeedsContainer)] = MapNeedsContainer, |
There was a problem hiding this comment.
This is an action that can be fired from within the handler to update the HasContainer.
| public override bool NeedsContainer => | ||
| VirtualView?.BackgroundColor != null || | ||
| base.NeedsContainer; |
There was a problem hiding this comment.
I also have to override the NeedsContainer property to control whether or not the handler is using a container or not.
| public static void MapBackgroundColor(ImageHandler handler, IImage image) | ||
| { | ||
| handler.UpdateValue(nameof(IViewHandler.ContainerView)); | ||
|
|
||
| handler.ContainerView?.UpdateBackgroundColor(image); | ||
| } |
There was a problem hiding this comment.
But, as a result, all the properties that need to work on the container are now redirected to the container instead of the default version that used the NativeView. In addition, we also have to let the handler know that how we might be changing the container, so first fire that and then we will have the container view to use.
I somehow feel that this could be changed... I was thinking that if the view has a container, then the base ViewHandler would use that for all the properties - for example: enabled, a11y, width, height - but this now means that if I add a background to my view, all the properties will need to be set on the new container and unset on the view. And it may be the case that setting a a11y property on a button handles differently when set on a border that contains a button. So for now, overriding the specific properties that need the container is not too bad. Maybe we need a way to mark properties (per handler) as a "this is a container property" and then if any of those are set the handler automatically creates a container.
There was a problem hiding this comment.
For A11y, there is accessibilityTraits on iOS and I don't think there is an equivalent concept for Android. For width and height, the container view should always be the same as the child view, no?
| #if WINDOWS | ||
| [nameof(ILabel.BackgroundColor)] = MapBackgroundColor, | ||
| #endif |
There was a problem hiding this comment.
This is what is needed now.
# Conflicts: # src/Core/src/Handlers/Label/LabelHandler.cs # src/Core/src/Platform/iOS/ViewExtensions.cs
|
|
||
| protected View? WrappedNativeView => ContainerView ?? (View?)NativeView; | ||
|
|
||
| public new WrapperView? ContainerView |
There was a problem hiding this comment.
What if, we rename ContainerView (currently used by Hot Reload) to ReloadableView and keep ContainerView for the Wrapper?
|
|
||
| DefaultTextColors = nativeView.TextColors; | ||
| DefaultPlaceholderTextColors = nativeView.HintTextColors; | ||
| DefaultBackground = nativeView.Background; |
There was a problem hiding this comment.
Could we add a handler test in these cases with custom code to assign the Background?
samhouts
added
area-image
ghost
added
the
legacy-area-controls
Eilon
removed
legacy-area-controls
samhouts
added
the
fixed-in-6.0.100-preview.5
5 participants
Description of Change
Many controls are "missing" features, so we have to wrap them into a container view that can support them.
This might be backgrounds on labels, clipping on unclippable views or even just a way to get a feature that can't work properly on the native view - such as gestures.
Additions made
object? IViewHandler.ContainerView { get; }NativeView? INativeViewHandler.ContainerView { get; }object? ViewHandler.NeedsContainer { get; }WrapperView : NativeViewPR Checklist
Does this PR touch anything that might affect accessibility?
If any of the above checkboxes apply to your PR, then the PR will need to provide testing to demonstrate that accessibility still works.