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.

Sign up for GitHub

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

Jump to bottom

Refactor page navigation #108

Merged
merged 7 commits into from
Apr 6, 2022
Merged

Refactor page navigation #108

merged 7 commits into from
Apr 6, 2022

Conversation

lucas-zimerman
Copy link
Collaborator

@lucas-zimerman lucas-zimerman commented Apr 5, 2022 •
edited
Loading

  • Add SentryOptions extension: RemoveNavigationPageIntegration
  • Isolated the logic to receive the Application.Current
  • refactored the Xamarin Forms integration, isolating the navigation integration from the rest of the Xamarin Forms integration.

Close #99.

@lucas-zimerman lucas-zimerman self-assigned this Apr 5, 2022
@lucas-zimerman lucas-zimerman added the enhancement New feature or request label Apr 5, 2022
mattjohnsonpint
mattjohnsonpint reviewed Apr 6, 2022
View reviewed changes
Src/Sentry.Xamarin.Forms/Internals/FormsApplicationListener.cs
/// </summary>
public void Invoke() =>
//Don't lock the main Thread while waiting for the current application to be created.
Task.Run(async () =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Yikes. I see this was in the old code and you are just refactoring, but we really shouldn't completely lose this task. It should be saved somewhere. We should also register it for cancellation so it can be shut down gracefully. I'm OK if this is done in a future PR, but wanted to point it out.

mattjohnsonpint
mattjohnsonpint approved these changes Apr 6, 2022
View reviewed changes
Copy link
Contributor

@mattjohnsonpint mattjohnsonpint left a comment

Choose a reason for hiding this comment

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

Approved with suggested changes.

@lucas-zimerman lucas-zimerman merged commit 6c15473 into main Apr 6, 2022
@lucas-zimerman lucas-zimerman deleted the feat/RemoveNavigationPageIntegration branch April 6, 2022 17:35
@lucas-zimerman lucas-zimerman mentioned this pull request Apr 6, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattjohnsonpint mattjohnsonpint mattjohnsonpint approved these changes

Assignees

@lucas-zimerman lucas-zimerman

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Define custom page types to be ignored
2 participants
@lucas-zimerman @mattjohnsonpint