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

Fix Scoping of MauiContext as it relates to Modal Pages #3361

Merged
merged 5 commits into from
Nov 18, 2021
Merged

Fix Scoping of MauiContext as it relates to Modal Pages #3361

merged 5 commits into from
Nov 18, 2021

Conversation

PureWeen
Copy link
Member

@PureWeen PureWeen commented Nov 11, 2021 •
edited
Loading

Description of Change

Setup Modal Pages with their own NavigationRoot on Windows and Android.

Remove the OnBackButtonPressing event and instead just uses OnBackButtonPressed to implement conditional behavior.

Remove the ctor on MauiContext that takes a parent MauiContext

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)
  • Targets a single property for a single control (or intertwined few properties)
  • Adds the property to the appropriate interface
  • Avoids any changes not essential to the handler property
  • Adds the mapping to the PropertyMapper in the handler
  • Adds the mapping method to the Android, iOS, and Standard aspects of the handler
  • Implements the actual property updates (usually in extension methods in the Platform section of Core)
  • Tags ported renderer methods with [PortHandler]
  • Adds an example of the property to the sample project (MainPage)
  • Adds the property to the stub class
  • Implements basic property tests in DeviceTests

Does this PR touch anything that might affect accessibility?

  • Does this PR introduce a new control? (If yes, add an example using SemanticProperties to the SemanticsPage)
  • APIs that modify focusability?
  • APIs that modify any text property on a control?
  • Does this PR modify view nesting or view arrangement in anyway?
  • Is there the smallest possibility that your PR will change accessibility?
  • I'm not sure, please help me

If any of the above checkboxes apply to your PR, then the PR will need to provide testing to demonstrate that accessibility still works.

PureWeen
PureWeen commented Nov 11, 2021
View reviewed changes
src/Core/src/Platform/Windows/WindowHeader.xaml
@@ -4,7 +4,7 @@
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
mc:Ignorable="d" Background="Purple"
mc:Ignorable="d"
Copy link
Member Author

Choose a reason for hiding this comment

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

🙄

@rmarinho
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@PureWeen PureWeen added the do-not-merge Don't merge this PR label Nov 12, 2021
@PureWeen PureWeen changed the title Fix Scoping of MauiContext as it relates to Modal Pages [WiP] Fix Scoping of MauiContext as it relates to Modal Pages Nov 12, 2021
Base automatically changed from nav_fixes to main November 15, 2021 16:17
@PureWeen PureWeen linked an issue Nov 15, 2021 that may be closed by this pull request
[Bug] Image is not always displayed when HeightRequest in not set on Android #2623
Closed
@mattleibow mattleibow marked this pull request as draft November 17, 2021 17:15
PureWeen
PureWeen commented Nov 17, 2021
View reviewed changes
src/Controls/src/Core/Page.cs
@@ -287,7 +287,9 @@ protected virtual bool OnBackButtonPressed()
var canceled = false;
EventHandler handler = (sender, args) => { canceled = true; };
window.PopCanceled += handler;
Navigation.PopModalAsync().ContinueWith(t => { throw t.Exception; }, CancellationToken.None, TaskContinuationOptions.OnlyOnFaulted, TaskScheduler.FromCurrentSynchronizationContext());
Navigation
Copy link
Member Author

Choose a reason for hiding this comment

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

The other way just swallows this exception and leaves you confused why nothing is happening

@PureWeen PureWeen changed the title [WiP] Fix Scoping of MauiContext as it relates to Modal Pages Fix Scoping of MauiContext as it relates to Modal Pages Nov 18, 2021
@PureWeen PureWeen removed the do-not-merge Don't merge this PR label Nov 18, 2021
@PureWeen PureWeen marked this pull request as ready for review November 18, 2021 02:57
@PureWeen PureWeen enabled auto-merge (squash) November 18, 2021 02:57
@PureWeen PureWeen merged commit 863dfe9 into main Nov 18, 2021
@PureWeen PureWeen deleted the modal_embbedded_nav_fixes branch November 18, 2021 03:35
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
@samhouts samhouts added the fixed-in-6.0.101-preview.11.3 Look for this fix in 6.0.101-preview.11.3! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jsuarezruiz jsuarezruiz Awaiting requested review from jsuarezruiz

@mattleibow mattleibow Awaiting requested review from mattleibow

Assignees
No one assigned
Labels
fixed-in-6.0.101-preview.11.3 Look for this fix in 6.0.101-preview.11.3!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] Image is not always displayed when HeightRequest in not set on Android
4 participants
@PureWeen @rmarinho @jsuarezruiz @samhouts