-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Correctly handle ContainerView changes #13114
base: main
Are you sure you want to change the base?
Conversation
@@ -14,4 +15,11 @@ public interface IViewHandler : IElementHandler | |||
|
|||
void PlatformArrange(Rect frame); | |||
} | |||
|
|||
public interface INeedsContainerViewHandler : IViewHandler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignore this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who knows if this is good, but it is a WIP.
public static void MapVisibility(IViewHandler handler, IView view) | ||
{ | ||
handler.Invoke(nameof(INeedsContainerViewHandler.NeedsContainer), nameof(IView.Visibility)); | ||
|
||
if (handler.HasContainer) | ||
{ | ||
((PlatformView?)handler.ContainerView)?.UpdateVisibility(view); | ||
((PlatformView?)handler.PlatformView)?.UpdateVisibility(Visibility.Visible); | ||
} | ||
else | ||
{ | ||
((PlatformView?)handler.PlatformView)?.UpdateVisibility(view); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a property changes that needs to respond to ContainerView changes, we call the "special" invoke and pass the property that requires a re-fire. This updates a collection, and ALSO triggers the path to ensure the container is correct.
Then based on the HasContainer, the mapper needs to set the values of the container and/or the platform view.
public static void MapNeedsContainer(IViewHandler handler, IView view, object? args) | ||
{ | ||
if (handler is ViewHandler viewHandler) | ||
handler.HasContainer = viewHandler.NeedsContainer; | ||
else | ||
handler.HasContainer = view.NeedsContainer(); | ||
if (handler is ViewHandler viewHandler && args is string mapper) | ||
viewHandler.ChangesContainer.Add(mapper); | ||
|
||
UpdateInputTransparentOnContainerView(handler, view); | ||
handler.UpdateValue(nameof(INeedsContainerViewHandler.NeedsContainer)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then the NeedsContainer command adds the incoming property to the list to update later. After that, it calls the NeedsContainer PROPERTY mapper.
public static void MapNeedsContainer(IViewHandler handler, IView view) | ||
{ | ||
#if ANDROID | ||
if (handler.ContainerView is WrapperView wrapper) | ||
wrapper.InputTransparent = view.InputTransparent; | ||
#endif | ||
if (handler is INeedsContainerViewHandler ncvh) | ||
handler.HasContainer = ncvh.NeedsContainer; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The NeedsContainer property mapper now updates the HasContainer property.
|
||
UpdateValue(nameof(IViewHandler.ContainerView)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HasContainer property adds/removes the container and then calls the ContainerView property mapper.
If the value is the same as before, it is terminated here and we skip the rest.
public static void MapContainerView(IViewHandler handler, IView view) | ||
{ | ||
if (handler is INeedsContainerViewHandler ncvh) | ||
{ | ||
foreach (var map in ncvh.ChangesContainer) | ||
handler.UpdateValue(map); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ContainerView then re-fires all the things that were added before.
handler.UpdateValue(nameof(IViewHandler.ContainerView)); | ||
(handler as IDynamicContainerViewHandler)?.ContainerAffectingProperties?.Add(nameof(IView.Clip)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new code now adds the property to the collection.
void OnChangedContainerCollectionChanged(object? sender, EventArgs e) => | ||
UpdateValue(nameof(IDynamicContainerViewHandler.NeedsContainer)); | ||
|
||
class CollectionWithEvents : ICollection<string> | ||
{ | ||
HashSet<string> properties = new(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The collection is special, and knows when a new property has been added. When a new property is added, it will fire the whole container view code.
So if you are adding 2 props that add a container, no need to go through the whole process if the ContainerView is already set - just use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM code wise.
Description of Change
Issues Fixed