Skip to content
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

Remove CastingList from MenuFlyoutSubItem #26558

Merged
merged 1 commit into from
Dec 12, 2024
Merged

Conversation

PureWeen
Copy link
Member

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 to IMenuElement

LogicalChildrenInternalBackingStore = new CastingList<Element, IMenuElement>(_menus);

Trying to add the IconImageSource to the LogicalChildrenInternalBackingStore insert a null value because this casting

_list.Add(item as TFrom);
fails, cannot convert from Element to IMenuElement.

This PR splits the implementation of the internal _menu list up from the logical children. This allows Logical Children of any type to get added.

Issues Fixed

Fixes #25893

@Copilot Copilot bot review requested due to automatic review settings December 11, 2024 23:07
@PureWeen PureWeen requested a review from a team as a code owner December 11, 2024 23:07
@PureWeen PureWeen added this to the .NET 9 SR3 milestone Dec 11, 2024
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 suggestions.

Comments skipped due to low confidence (1)

src/Controls/src/Core/Menu/MenuFlyoutSubItem.cs:65

  • The insertion of the item to _menus should occur after inserting it as a logical child to ensure the logical child index matches the _menus list index.
_menus.Insert(index, item);

@rmarinho rmarinho merged commit a564456 into main Dec 12, 2024
110 checks passed
@rmarinho rmarinho deleted the fix-25893-alternate branch December 12, 2024 12:02
@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Dec 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-net8.0-nightly This may be available in a nightly release! fixed-in-net9.0-nightly This may be available in a nightly release!
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Setting MenuFlyoutSubItem IconImageSource throws a NullReferenceException
3 participants