-
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
[Windows] Fix crash setting MenuFlyoutSubItem IconImageSource #26021
Conversation
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.
Can you use a unit test here instead of a UI test?
Added both, xlml test and UITest. |
@@ -87,7 +82,7 @@ public void RemoveAt(int index) | |||
{ | |||
var item = _menus[index]; | |||
RemoveLogicalChild((Element)item, index); | |||
NotifyHandler(nameof(IMenuFlyoutHandler.Remove), index, item); | |||
NotifyHandler(nameof(IMenuFlyoutHandler.Remove), index, item as IMenuElement); |
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.
I don't know if this will work...
I feel like we've hit a point on this one where we actually need to maintain two different internal lists.
_menus
no longer represents _menus
it's a mix of types.
If someone calls RemoveAt(index)
they are going to expect that it removes the menu specifically at the given index (we should also make sure this removes the ImageSource added)
So, I think we have to keep _menus
as a list of IMenuElement to satisfy all the IList<IMenuElement>
implementations.
And then the backing store for logical children basically has to be a mix of both.
We basically do this with Layout.cs
, it has an internal list of _children
it uses to satisfy the direct children but then it adds each of this to the logical children on the parent
Description of Change
Fix crash setting
MenuFlyoutSubItem
IconImageSource on Windows.From #24688 the ImageSource is added as logical children of the menu item. However, the
LogicalChildrenInternalBackingStore
collection from MenuFlyoutSubItem does a casting toIMenuElement
maui/src/Controls/src/Core/Menu/MenuFlyoutSubItem.cs
Line 19 in 5b5aaa5
Trying to add the IconImageSource to the LogicalChildrenInternalBackingStore insert a null value because this casting
maui/src/Controls/src/Core/ReadOnlyCastingList.cs
Line 91 in 5b5aaa5
IMenuElement
.This PR apply changes to allow Element in the
LogicalChildrenInternalBackingStore
collection from MenuFlyoutSubItem.Issues Fixed
Fixes #25893