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

Fix For Maui WinUI Registrations #3577

Merged
merged 2 commits into from Jul 8, 2023

Conversation

ChrisPulman
Copy link
Member

Fix for #3576

What kind of change does this PR introduce?

Fix for #3576

What is the current behaviour?

ActivationForViewFetcher is not registered

What is the new behaviour?

Maui and WinUI Registrations are now Registered
Namespace issue resolved

What might this PR break?

None

Please check if the PR fulfils these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

@@ -28,12 +33,15 @@ public class ActivationForViewFetcher : IActivationForViewFetcher
/// <inheritdoc/>
public int GetAffinityForView(Type view) =>
#if HAS_WINUI
#if IS_MAUI
Copy link
Member

Choose a reason for hiding this comment

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

Should IS_MAUI be nested in the HAS_WINUI here?


#if HAS_MAUI
#if HAS_MAUI || (HAS_WINUI && IS_MAUI)
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference between HAS_MAUI and IS_MAUI? Should we rename these?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are to distinguish the Maui Android / iOS section (HAS_MAUI) and the Maui WinUI section HAS_WinUI, as we are using a single file between the two projects (Maui and WinUI) the namespaces need to match the assembly hence the IS_MAUI and IS_WINUI relates to the Project SDK values of UseMaui and UseWinUI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way for us to have more meaningful name for one of them. Don't mind a bit of verbosity.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can look at dropping HAS_MAUI and invert HAS_WINUI where required, perhaps make HAS_WINUI => WINUI_TARGET

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah seems nicer name.

@codecov
Copy link

codecov bot commented Jul 8, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.02 ⚠️

Comparison is base (9d73fba) 64.09% compared to head (f481c56) 64.07%.

❗ Current head f481c56 differs from pull request most recent head 87fa248. Consider uploading reports for the commit 87fa248 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3577      +/-   ##
==========================================
- Coverage   64.09%   64.07%   -0.02%     
==========================================
  Files         157      157              
  Lines        5762     5762              
==========================================
- Hits         3693     3692       -1     
- Misses       2069     2070       +1     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ChrisPulman ChrisPulman enabled auto-merge (squash) July 8, 2023 13:54
@ChrisPulman ChrisPulman merged commit 3467c36 into main Jul 8, 2023
1 check passed
@ChrisPulman ChrisPulman deleted the FixForMauiWinUIReactiveUIRegistrations branch July 8, 2023 14:06
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 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

3 participants