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 Microsoft.Maui.Controls.Compatibility Dependency #2006

Merged
merged 9 commits into from
Jul 23, 2024

Conversation

filipnavara
Copy link
Contributor

Description of Change

Replace one extension with another one to reduce dependencies on compatibility package.

Linked Issues

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

Additional information

@brminnick brminnick marked this pull request as ready for review July 18, 2024 18:10
@brminnick brminnick changed the title Replace Color.ToAndroid (MAUI compatibility) with Color.ToPlatform (MAUI core) Remove Microsoft.Maui.Controls.Compatibility Dependency Jul 18, 2024
@brminnick
Copy link
Collaborator

brminnick commented Jul 18, 2024

Hey @JoonghyunCho! Could you help me fix this error on the Tizen implementation of MediaElement?

D:\a\1\s\src\CommunityToolkit.Maui.MediaElement\Views\MediaManager.tizen.cs(183,41): error CS0103: The name 'ResourcePath' does not exist in the current context

https://dev.azure.com/dotnet/CommunityToolkit/_build/results?buildId=107977&view=logs&j=67da6a11-18ef-5f70-9b43-fae7e6c83e18&t=f6318a24-ba44-5d24-7d56-2bdec230cbb2&l=1073

We're looking to remove our dependency on Microsoft.Maui.Controls.Compatibility, and it looks like ResourcePath.GetPath() is no longer available in .NET MAUI without the compatibility NuGet Package.

Could you see about retrieving the path to the Resources folder on Tizen without using the ResourcePath class?

@filipnavara
Copy link
Contributor Author

Note the the <UseMaui>true</UseMaui> property still brings in the dependency. That needs to be removed and we need to fix the iOS build of TouchBehavior.

@brminnick
Copy link
Collaborator

brminnick commented Jul 19, 2024

we need to fix the iOS build of TouchBehavior

What's the problem we need to fix for this?

I'll the Do Not Merge label until we've covered everything.

@brminnick brminnick added the do not merge Do not merge this PR label Jul 19, 2024
@filipnavara
Copy link
Contributor Author

What's the problem we need to fix for this?

if (((platformView as IVisualNativeElementRenderer)?.Control ?? platformView) is UIButton button)
{
button.AllTouchEvents += HandleAllTouchEvents;
((TouchUITapGestureRecognizer)touchGesture).IsButton = true;
}

IVisualNativeElementRenderer comes from the compatibility package. Now I would argue that it has no business being there and it was just copied from the Xamarin.CommunityToolkit, but it may be more complex. If we want to support it on compatibility controls that use UIButton then it may need some additional changes/tests.

@brminnick brminnick added approved This Proposal has been approved and is ready to be added to the Toolkit and removed do not merge Do not merge this PR labels Jul 23, 2024
brminnick
brminnick previously approved these changes Jul 23, 2024
Copy link
Collaborator

@brminnick brminnick left a comment

Choose a reason for hiding this comment

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

Thanks @filipnavara!!

@brminnick brminnick merged commit f9519ba into CommunityToolkit:main Jul 23, 2024
8 checks passed
@filipnavara
Copy link
Contributor Author

Thanks for pushing it through and fixing the last bits! Much appreciated!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved This Proposal has been approved and is ready to be added to the Toolkit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants