Unregister the Loaded event in Relative Ancestor

[michael-hawker]

Helps with CommunityToolkit/Windows#107

TokenizingTextBox uses this for the TokenSpacing property, so tested quick with that and seems to still work fine.

PR Type

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

• Tested code with current supported SDKs
• New component
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • If control, added to Visual Studio Design project
• Sample in sample app has been added / updated (for bug fixes / features)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• New major technical changes in the toolkit have or will be added to the Wiki e.g. build changes, source generators, testing infrastructure, sample creation changes, etc...
• Tests for the changes have been added (for bug fixes / features) (if applicable)
• Header has been added to all new source files (run build/UpdateHeaders.bat)
• Contains NO breaking changes

Other information

[ghost]

Thanks michael-hawker for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

[Nirmal4G]

This seems like a common code. If only TokenizingTextBox uses this then it's fine else we need to evaluate this behavioral change across all controls which use this!?

This could be breaking change if anyone outside of the Toolkit uses these extensions.

[michael-hawker]

@Nirmal4G that's the only place we use it in the Toolkit which is easy to test, but I'm unsure of the larger ramifications on this if used in something that may be unloaded and re-loaded. This whole scenario is getting complicated, this was only ever meant as a stop-gap. Will need to check if we've got an issue tracking this on WinUI, I know it's certainly related to some x:Bind gaps we've filed.

Will move this out of the hotfix for now so we can think about a restructure for this scenario moving forward.

[Nirmal4G]

…I'm unsure of the larger ramifications on this if used in something that may be unloaded and re-loaded…

I can think of some ways that this could be breaking if used externally. Anyhow this unload seems like a best solution as of now. We could add this to a next preview release, gather feedback and incorporate this if there's liitle or no friction against this!

…we can think about a restructure for this scenario moving forward.

I agree! Hopefully in the next major/minor release maybe? 🧐

[michael-hawker]

Closing as new PR here taking over: CommunityToolkit/Windows#105

Investigate and didn't see impact on tests. This should be an event on the object owning the event, so we should be OK. We do want to account for something unloading and loading again, so we probably want to keep the event reference.