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

WinUI Navigation Handler #801

Merged
merged 6 commits into from
Apr 26, 2021
Merged

WinUI Navigation Handler #801

merged 6 commits into from
Apr 26, 2021

Conversation

PureWeen
Copy link
Member

@PureWeen PureWeen commented Apr 21, 2021

Description of Change

Most of the file changes have to do with namespace changes because of types moved into controls from compatibility and then renaming WINDOWS to WINDOWS

The good bits are all inside the Controls.Core and the Core project
That's the main code to focus on

Changes

Moving files from Compatibility to Controls.Core

  • if the type/interface (ITitleProvider) is internal then I just copied the file over which means there are two copies
  • If the type/interface is public then I moved it from Compatibility to Controls.Core
  • If the Extension class is public then for the most part I moved it to Controls.Core
    • A couple cases ColorExtensions.ToFormsColor and AccessibilityExtensions I split the file up where the extension method would no longer make sense in Controls.Core
  • Public class types (VisualElementChangedEventArgs) I moved to Controls.Core so Compat/Core worked off the same type
  • Xaml ControlTemplates - I copy/pasted these over and renamed them from "Forms" to "Maui".
  • I updated the CommandBar template to match the current WinUI template. The template we have now was generated from 12041 6 years ago. We'll want to validate against some UI tests once we are up and running in the CG (Allow CommandBar to expand and show command labels xamarin/Xamarin.Forms#594)
  • Moved the Platform.UpdateToolBarItems into ToolbarManager.UpdateToolBarItems (This will need to be updated to handle multiple windows)

Observations

  • every place we use async void should be verified when moved over
    • does it try catch? (use fireandforget)
    • does it validate that the handler is still in a state that the results are valid?
    • Is there a way to make this all more mapper friendly so we don't need async/void? Once we get the new image stuff a lot of the async/void should be able to go away
  • I tried to use IView instead of VisualElement wherever I could but we will probably try to have a later effort to really take this one home
  • I'll add Issues for all the "//TODO MAUI" stuff
  • There are some APIs that will only work on UWP which I have left around on the assumption that some day we're going to have a UWP WinUI target so I don't want to delete that code

Additions made

  • I added a Task.FireAndForget into Core that try/catches for async/void
  • Added UseMauiControlsHandlers builder extension which will register all the maui controls handlers and whatever other types are needed
  • I put all of the platform specific bits under the "MS.Maui.Controls.Platform" Namespace
  • I collapsed all of the Windows Windows_UWP if defs just into "WINDOWS" and I removed any UWP specific code because this code isn't relevant anymore. I realize earlier on I said I left some UWP code but that was only for scenarios where the API is available to UWP. The area's I removed should have APIs that match WinUI desktop if/once uwp comes around

Removals made

  • Bits that I moved over to Controls that work marked as hidden from intellisense I marked as internal. Because we are just going to be compiling the handlers along side the Controls.Core in the same assembly they don't have to cross the assembly line anymore

Testing

WinUI still doesn't draw without a window resize so you have to resize the window, then you can click the navigate button, then resize the window again to see the new page

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?

  • 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 marked this pull request as ready for review April 21, 2021 22:58
@PureWeen PureWeen changed the title [WiP] Winui navigation WinUI Navigation Handler Apr 21, 2021
string path;

#if WINDOWS_UWP
path = Windows.Storage.ApplicationData.Current.LocalFolder.Path;
Copy link
Member Author

Choose a reason for hiding this comment

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

AFAICT this code never executed even on old forms

Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

Wow... Big one. Most look OK, and I am going to trust that because a bunch of the code is copied form the renderer that this is safe to use. I got a few questions more along the line of "what is this doing as I don't understand?"

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

The usage of the extension methods as normal methods is a little sus :) But no biggie.

Let's merge this one and then we can get nav working

@PureWeen PureWeen merged commit efa9dab into main Apr 26, 2021
@PureWeen PureWeen deleted the winui_navigation branch April 26, 2021 23:21
PureWeen added a commit that referenced this pull request Apr 27, 2021
* WinUI Navigation Handler

* - PR comments

* - consolidate a few more APIs

* - fix namespaces

* - fix namespaces

* Update src/Compatibility/Core/src/WinUI/FlyoutPageRenderer.cs

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
rmarinho pushed a commit that referenced this pull request Apr 27, 2021
* WinUI Navigation Handler (#801)

* WinUI Navigation Handler

* - PR comments

* - consolidate a few more APIs

* - fix namespaces

* - fix namespaces

* Update src/Compatibility/Core/src/WinUI/FlyoutPageRenderer.cs

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>

* Improve Hot Reload Integration (#851)

* Fixes HotReload (#853)

Reverts changed from: 7d4d1ce#diff-2b4bb95e12307423f7126d91b43a1511b68d00535e9f2b5f535af85d7a388333R36

* Android Push/Pop Navigation (#837)

* Android Navigation

* - fix layout

* - changes

* - yay pushing

* - wire up nav to use bundle ids

* - cleanup

* - setup pop

* - build up nav stack

* - remove nav push

* - remove graph xml

* Update global.json

* - add android to non net6 controls project

* - fix namespace

* Update Microsoft.Maui-net6.sln

* iOS NavigationPageHandler (#852)

* iOS Navigation

* - wire up iOS Navigation

* - remove VET

* - rework with HR

* - fix namespace

* - fix HR

* - fix hr

* - fix SO exception

* - maybe?

* - nullabel fix

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: James Clancey <james.clancey@gmail.com>
Redth added a commit that referenced this pull request Apr 28, 2021
* Fix yaml triggers

* Run pr's for release branches

* Automated dotnet-format update (#839)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* [Housekeeping] Add provisionating xcode to net6 (#836)

* Add provisionating xcode to net6

* Fix essentials device tests

* try again

* Update dependencies from https://github.com/xamarin/xamarin-android build main-a2156d41275b92de3cf23f7c79801327c74b7fe0-1 (#846)

Microsoft.Android.Sdk.Windows
 From Version 11.0.200-ci.main.226 -> To Version 11.0.200-ci.main.234

Dependency coherency updates

Microsoft.Dotnet.Sdk.Internal
 From Version 6.0.100-preview.4.21215.1 -> To Version 6.0.100-preview.4.21221.10 (parent: Microsoft.Android.Sdk.Windows

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* WinUI Navigation Handler (#801)

* WinUI Navigation Handler

* - PR comments

* - consolidate a few more APIs

* - fix namespaces

* - fix namespaces

* Update src/Compatibility/Core/src/WinUI/FlyoutPageRenderer.cs

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>

* Improve Hot Reload Integration (#851)

* Fixes HotReload (#853)

Reverts changed from: 7d4d1ce#diff-2b4bb95e12307423f7126d91b43a1511b68d00535e9f2b5f535af85d7a388333R36

* Android Push/Pop Navigation (#837)

* Android Navigation

* - fix layout

* - changes

* - yay pushing

* - wire up nav to use bundle ids

* - cleanup

* - setup pop

* - build up nav stack

* - remove nav push

* - remove graph xml

* Update global.json

* - add android to non net6 controls project

* - fix namespace

* Update Microsoft.Maui-net6.sln

* iOS NavigationPageHandler (#852)

* iOS Navigation

* - wire up iOS Navigation

* - remove VET

* - rework with HR

* - fix namespace

* - fix HR

* - fix hr

* - fix SO exception

* - maybe?

* - nullabel fix

* Update dependencies from https://github.com/xamarin/xamarin-macios build 20210426.5 (#847)

Microsoft.MacCatalyst.Sdk , Microsoft.iOS.Sdk
 From Version 14.5.100-ci.main.620 -> To Version 14.5.100-ci.main.658

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Rui Marinho <me@ruimarinho.net>

* Automated dotnet-format update (#854)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Implement PickerHandler in WinUI (#779)

* Implement PickerHandler in WinUI

* Enab le nullable in some classes

* Fix build error

* Register MauiComboBox ResourceDictionary

* Updated PickerExtensions

* Updated MauiComboBox

Co-authored-by: Rachel Kang <rachelkang@microsoft.com>

* [Build] Update maestro with macOS and tvOS (#862)

* [Build] Update maestro with macOS and tvOS

* [Build] Add tvOS and macOS to workload/dogfood

* Add Microsoft.iOS.Windows.Sdk

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>

* Update README.md (#865)

* ContainerViewControllers should use the correct background color. (#868)

Also set the title of the VC from a page

* Automated dotnet-format update (#872)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Bring back XAML2006/2009 XmlnsDefinition attributes

These are required so that things like x:Static can be resolved to Microsoft.Maui.Controls.Xaml.StaticExtension.

Co-authored-by: Rui Marinho <me@ruimarinho.net>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: James Clancey <james.clancey@gmail.com>
Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
Co-authored-by: Rachel Kang <rachelkang@microsoft.com>
Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
Co-authored-by: 1d0n7kn0w <3910210+1d0n7kn0w@users.noreply.github.com>
Co-authored-by: Jonathan Dick <jodick@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Dec 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants