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 #2050 [DOC] Update Sample Application to demonstrate correct way to initialize Maps in MauiProgram.cs to support Maps on all platforms. #2325

Merged
merged 3 commits into from
Nov 8, 2024

Conversation

mikelor
Copy link
Contributor

@mikelor mikelor commented Nov 7, 2024

Description of Change

Demonstrate the correct way to incorporate .Net Maui Maps for Windows into a multiplatform application.

The 2.0.1 Release of the Community Toolkit Maps component introduced a change that requires the #ifdef Windows directive when compiling a multiple platform application with Maps.

This can cause confusion for people trying to incorporate Maps into the application due to the fact that since Maui 8.0.10 now throws an exception on Windows when using the standard Maps component.

No prior tests for this sample code.
This code is a sample.

Linked Issues

PR Checklist

Additional information

Modified ReadMe.txt to illustrate proper way instantiate .Net Maui Maps when using CommunityToolk.Maui.Maps

Tested on Android API 35 (requires Google API Key) - New Requirement
Tested on Windows 11

Did not test on iOS or Mac Catalyst

Copy link
Contributor

@bijington bijington left a comment

Choose a reason for hiding this comment

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

@mikelor thank you for making these changes. I have some questions though:

  1. Should we add this into the docs repository also?
  2. Should we add your code example into the XML docs for the UseCommunityToolkitMaps extension method?
  3. I understand the desire to out into the readme file and others might agree but I suspect maps is not a commonly used feature to have it in this default content. What do others think?

@mikelor
Copy link
Contributor Author

mikelor commented Nov 7, 2024

@bijington the ReadMe.Txt modified in this PR is specific to the Maps assembly. See the current version here

I'd be happy to take a look at the XML docs and add an explanation there as well. Here's what was in the Release Notes for the 2.01 Version that caused my frustration. If the sample was updated or it was in the XML docs for the method, I may have resolved my issue earlier. LoL.

@bijington
Copy link
Contributor

That'll teach me for reviewing the changes on my phone and missing that detail. Apologies for that.

That would be brilliant if you could. Thank you

@mikelor
Copy link
Contributor Author

mikelor commented Nov 7, 2024

Ok @bijington, when you get a chance take a look. I added some example code to the XML docs. They generated just fine.

Copy link
Contributor

@bijington bijington left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you so much @mikelor

@bijington bijington merged commit d42bd2a into CommunityToolkit:main Nov 8, 2024
9 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 18, 2024
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.

2 participants