Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Initial implementation of Segues #2816

Closed
wants to merge 5 commits into from
Closed

Conversation

chkn
Copy link

@chkn chkn commented May 24, 2018

Description of Change

Segues are objects that represent navigational actions. These objects can be created and wired up entirely in XAML without any codebehind (or with code, if you prefer).

This PR is an evolution of https://github.com/chkn/Xamarin.Forms.Segues integrated into Core. This integration has some benefits:

  • Integrates into navigation
    • New NavigationAction enum maps to INavigation methods
    • New internal type, ValueSegue: a lightweight struct that holds either a simple NavigationAction or a full Segue instance
    • A lot of navigation internals have been refactored in terms of these, which I believe simplifies and streamlines things.
  • Pay only for what you use
    • Overloads of new methods are provided that take NavigationAction when a full Segue instance is not needed
    • {Segue} markup extension does not actually instantiate Segue (returns a lightweight ICommand that triggers the same behavior)
    • Full Segue instance can be provided by user if they want to enable/disable it, subclass it, bind properties, or to perform actions when the segue is executed.
  • Ultimately, will enable custom transitions (to come later in a separate PR)

I've included the platform implementation for iOS only for now. If we decide we want to proceed with this approach, I could implement Android and UWP as well.

NavigationAction

This introduces the NavigationAction enum, whose values mostly represent method calls on INavigation. However, there are a couple differences:

Show

This PR introduces INavigation.ShowAsync and NavigationAction.Show. This action is designed to do the right thing in most common cases, and as such, is the default NavigationAction for segues. If the user does not want this default behavior, they can specify one of the other navigation actions explicitly.

Behavior depends on the context. Here are the rules:

  • If the source page's ancestor is a NavigationPage, calls INavigation.PushAsync.
  • Otherwise, calls INavigation.PushModalAsync.

I also had a couple other interesting ideas for this action in the context of a MultiPage or MasterDetailPage, but I did not implement them for this PR.

Caveat: Throws an InvalidOperationException if this action is used when navigation is not rooted (i.e. NavigationProxy.Inner == null). It might be possible to remove this limitation, but there are some issues that would need to be resolved.

Pop

This PR also introduces NavigationAction.Pop. Note that this is NOT the equivalent of calling INavigation.PopAsync, which is an existing API that corresponds to the new NavigationAction.PopPushed.

Like the above, NavigationAction.Pop tries to do the right thing depending on the context:

  • If there is anything on the modal stack, call INavigation.PopModalAsync.
  • Otherwise, call INavigation.PopAsync.
MainPage

Sometimes you either don't want stack-based navigation, or you want to reset the root of your app easily. As such, NavigationAction.MainPage is included that trivially sets Application.Current.MainPage to the target page.

Native Segues

Segues may be used to navigate to native screens. This functionality is analogous to the native view support (https://docs.microsoft.com/en-us/xamarin/xamarin-forms/platform/native-views/).

Examples

Attach a segue to a button from XAML

<Button Text="Go to next page"
        Command="{Segue}"
        Segue.Target="{x:Type local:NextPage}" />

Note that Segue.Target can also be a DataTemplate.

Attach a segue to a button from code

// Attaches a segue to be executed when the button is clicked
button.SetSegue(NavigationAction.Show, new DataTemplate(typeof(NextPage)));

Imperatively perform a segue from code

// Executes the segue now
await Navigation.NavigateAsync(NavigationAction.Show, new NextPage());

Bugs Fixed

None

API Changes

Added:

namespace Xamarin.Forms
{
    // The following public types have been added:
    public enum NavigationAction
    public class NavigationActionConverter : TypeConverter
    public class Segue : BindableObject, ISegueExecution
    public class SegueBeforeExecuteEventArgs : EventArgs
    public static class SegueExtensions
    public class SegueTarget

    // The following members have been added:
    Task INavigation.ShowAsync(Page page);
    Task INavigation.ShowAsync(Page page, bool animated);
    Task INavigation.SegueAsync(Segue segue, SegueTarget target);

    public event EventHandler<SegueRequestedEventArgs> NavigationPage.SegueRequested;

    public Task NavigationProxy.ShowAsync(Page page)
    public Task NavigationProxy.ShowAsync(Page page, bool animated)
    public Task NavigationProxy.SegueAsync(Segue segue, SegueTarget target)
}

namespace Xamarin.Forms.Internals
{
    // The following public types have been added:
    public interface ICommandableElement
    public interface ISegueExecution
    public class SegueRequestedEventArgs
    public struct ValueSegue
}

namespace Xamarin.Forms.Xaml
{
    public class SegueExtension : IMarkupExtension<ICommand>
}

namespace Xamarin.Forms.Platform.iOS
{
    // The following public types have been added:
    public class NativePageWrapper
    public class NativePageWrapperRenderer : IVisualElementRenderer
    public sealed class ViewControllerSegueTarget : SegueTarget

    // The following members have been added:
    public static Page PageExtensions.ToPage(this UIViewController viewController)
}

Changed:

namespace Xamarin.Forms
{
    // The following public types now implement `Xamarin.Forms.Internals.ICommandableElement`:
    public class Button : ICommandableElement
    public class TextCell : ICommandableElement
    public class ClickGestureRecognizer : ICommandableElement
    public class Entry : ICommandableElement
    public class SearchBar : ICommandableElement
    public class MenuItem : ICommandableElement
    public class TapGestureRecognizer : ICommandableElement
}

Behavioral Changes

None intended.

PR Checklist

  • Has tests (if someone can point me at similar tests, I will add some)
  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense

Alex Corrado added 5 commits May 23, 2018 15:38
…ranslate to a Page

This vastly simplifies things.

Note: this includes an iOS renderer, but there seems to be a bug with it that will need
to be addressed.
This added extra complexity, but its usefulness wasn't very obvious.

If a good argument were made for having this property, we can revert
this.
@dnfclas
Copy link

dnfclas commented May 24, 2018

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ chkn sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

@StephaneDelcroix StephaneDelcroix self-assigned this May 28, 2018
@davidortinau davidortinau added this to the 3.2.0 milestone Jun 11, 2018
@samhouts samhouts added the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Jun 26, 2018
@samhouts samhouts removed this from the 3.2.0 milestone Jul 23, 2018
@samhouts samhouts removed the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Jul 23, 2018
Copy link
Member

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

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

I'll pull this branch, rebase and review further soon

{
// implementing classes must be castable to Element
[EditorBrowsable(EditorBrowsableState.Never)]
public interface ICommandableElement
Copy link
Member

Choose a reason for hiding this comment

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

keep it internal

Copy link
Member

Choose a reason for hiding this comment

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

also, the BP can be extracted in a static helper class (see ITextElement, TextElement)

PopToRoot = Pop | MainPage,
}

public class NavigationActionConverter : TypeConverter
Copy link
Member

Choose a reason for hiding this comment

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

attribute with the return type

namespace Xamarin.Forms
{
[TypeConverter(typeof(NavigationActionConverter))]
public enum NavigationAction
Copy link
Contributor

Choose a reason for hiding this comment

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

Because the PopX are ORed together, shouldn't they all be powers of two (and be [Flags])? Although as it turns out everything will be fine in this case with them having values of 5, 6, 7 respectively.

Since the PopX values are special, and NavigationActions can't arbitrarily be ORed anyway, it seems better to specifically assign values to them.

@jassmith
Copy link

This is going to get integrated into Shell and at that point we can move it over into core as one block

@jassmith jassmith closed this Aug 30, 2018
@samhouts samhouts added this to the 3.2.0 milestone Sep 11, 2018
@samhouts samhouts removed this from the 3.2.0 milestone Sep 12, 2018
@chkn chkn mentioned this pull request Jan 7, 2019
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.

8 participants