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 null forgiveness operator and fix NRE from PlatformView in disconnect #17248

Merged
merged 1 commit into from Sep 7, 2023

Conversation

PureWeen
Copy link
Member

@PureWeen PureWeen commented Sep 7, 2023

Description of Change

In the disconnect part of the ShellItemHandler we were referring to the wrong platformview. This was further enabled by the use of a null forgiveness operator. I've removed the null forgiveness code, so that each use needs to be account for a possible null platformview.

@PureWeen PureWeen requested a review from a team as a code owner September 7, 2023 07:32
}

protected override void DisconnectHandler(FrameworkElement platformView)
{
base.DisconnectHandler(platformView);

ShellItemNavigationView.SelectionChanged -= OnNavigationTabChanged;
if (platformView is MauiNavigationView mnv)
mnv.SelectionChanged -= OnNavigationTabChanged;
Copy link
Member Author

Choose a reason for hiding this comment

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

This was the main fix

Copy link
Member

@jfversluis jfversluis left a comment

Choose a reason for hiding this comment

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

LGTM

@PureWeen PureWeen enabled auto-merge (squash) September 7, 2023 07:56
@PureWeen PureWeen merged commit 002d4aa into main Sep 7, 2023
38 checks passed
@PureWeen PureWeen deleted the fix_shellitemhandler_disconnect branch September 7, 2023 12:40
@github-actions github-actions bot locked and limited conversation to collaborators Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants