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 options not working for Core #2491

Merged
merged 7 commits into from
Feb 4, 2025

Conversation

ne0rrmatrix
Copy link
Contributor

  • Bug fix

Description of Change

Currently you cannot opt out of using new functionality for dialogue fragments on android. Setting the option to do so does not work with core. This PR fixes that issue by placing setting options earlier in build order.

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

This will allow ppl to opt out of using the new dialogue fragment in recent PR. Atm if you try to navigate to any page in sample app the app crashes to android desktop. This PR will fix that issue by allowing you to opt out. Current the opt out is not working. This PR addresses that issue only. It fixes options so that they work by placing options above core in build order.

@ne0rrmatrix ne0rrmatrix requested a review from pictos February 4, 2025 13:32
@ne0rrmatrix ne0rrmatrix added the area/core Issue/Discussion/PR that has to do with the core of the toolkit label Feb 4, 2025
TheCodeTraveler
TheCodeTraveler previously approved these changes Feb 4, 2025
Copy link
Collaborator

@TheCodeTraveler TheCodeTraveler left a comment

Choose a reason for hiding this comment

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

Thanks James! Good catch!!

@TheCodeTraveler TheCodeTraveler enabled auto-merge (squash) February 4, 2025 17:36
pictos
pictos previously approved these changes Feb 4, 2025
…ptionCompleted` to allowunit tests confirm that we initialized the AndroidDialogFragmentService
Copy link
Collaborator

@TheCodeTraveler TheCodeTraveler left a comment

Choose a reason for hiding this comment

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

Thanks again James! Just FYI - I added some Unit Tests to help us ensure we don't ever accidentally flip the order of operations again.

In other words, these unit tests ensure that we always invoke options?.Invoke(new Options(builder)); before we invoke builder.UseMauiCommunityToolkitCore(null);.

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

@TheCodeTraveler TheCodeTraveler merged commit 70db45d into CommunityToolkit:main Feb 4, 2025
10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/core Issue/Discussion/PR that has to do with the core of the toolkit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Options has incorrect Build Order
3 participants