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

[XF] Introduce a Behavior factory into the navigation service #1099

Closed
powerdude opened this issue Jun 27, 2017 · 7 comments · Fixed by #1245
Closed

[XF] Introduce a Behavior factory into the navigation service #1099

powerdude opened this issue Jun 27, 2017 · 7 comments · Fixed by #1245
Labels

Comments

@powerdude
Copy link

I'd like to try to tease apart the behavior injection inside the navigation service and introduce an IBehaviorFactory, or IPageFactory, so that it can be a bit easier to override or add additional functionality when standard XF pages are created.

@dansiegel
Copy link
Member

@powerdude I believe the functionality that you're looking for can already be accomplished by implementing a custom PageNavigationService and overriding ApplyPageBehaviors.

protected override void ApplyPageBehaviors(Page page)
{
    base.ApplyPageBehaviors(page);
    // Add any additional behaviors
}

@powerdude
Copy link
Author

@dansiegel , yes, it is possible there, but thought it would be more explicit to call out the functionality separately. There's a lot going on in that class and it does more than navigation and should probably rely on other, more specialized services to help it do what it does.

@brianlagunas
Copy link
Member

This is something that has been discussed in the past, but hasn't had any traction because it is not very common and can be accomplished by deriving from PageNavigationService like Dan suggested. If you would like to talk about a design and put a preliminary PR together for what you had in mind, that would be great.

@brianlagunas
Copy link
Member

I worked on a branch with this new abstraction, but I'm not sure if it is worth it or not. If I pull this out into an IPageBehaviorFactory we will have to rely on reflection in order to determine if the page type is the target type or a derived type in order to properly apply the behaviors. Reflection means performance hit. This code would invoke for every page created.

Essentially we would have this class

    public class PageBehaviorFactory : IPageBehaviorFactory
    {
        List<KeyValuePair<Type, Behavior>> _pageBehaviors = new List<KeyValuePair<Type, Behavior>>();

        public PageBehaviorFactory()
        {

        }

        public void Register<T>(Behavior behavior) where T : Page
        {
            _pageBehaviors.Add(new KeyValuePair<Type, Behavior>(typeof(T), behavior));
        }

        public IEnumerable<Behavior> GetPageBehaviors(Page page)
        {
            List<Behavior> behaviors = new List<Behavior>();

            foreach (var kvp in _pageBehaviors)
            {
                if (IsSameOrSubclass(kvp.Key, page.GetType()))
                {
                    behaviors.Add(kvp.Value);
                }
            }

            return behaviors.AsEnumerable();
        }

        bool IsSameOrSubclass(Type potentialBase, Type potentialDescendant)
        {
            return potentialDescendant == potentialBase || potentialDescendant.GetTypeInfo().IsSubclassOf(potentialBase);
        }
    }

Then you can register your behaviors in the Prism Application:

        protected virtual void ConfigurePageBehaviors(IPageBehaviorFactory pageBehaviors)
        {
            if (pageBehaviors != null)
            {
                pageBehaviors.Register<NavigationPage>(new NavigationPageSystemGoBackBehavior());
                pageBehaviors.Register<TabbedPage>(new TabbedPageActiveAwareBehavior());
                pageBehaviors.Register<CarouselPage>(new CarouselPageActiveAwareBehavior());
            }
        }

Thoughts?

@brianlagunas
Copy link
Member

I am closing this as there has not been a community response in a month.

@dansiegel
Copy link
Member

After thinking about this some more I believe that this would actually be a good addition. While you could override ApplyPageBehaviors as I mentioned above, this would require users to properly update the registration for INavigationService. Considering the general importance of INavigationService I think it would be better to introduce a simple factory that would allow users to apply any behaviors that they want.

public interface IPageBehaviorsFactory
{
    void ApplyPageBehaviors(Page page);
}

// this then simplifies registration

Container.Register<IPageBehaviorsFactory, PrismPageBehaviorsFactory>(Reuse.Singleton);

With the addition of Dynamic Tab Creation, it will also be more common to generically register a TabbedPage as well as a NavigationPage. This means that there are going to be use cases where you simply want to add a PlatformSpecific. There shouldn't be a need to add a custom TabbedPage for example just to add some platform specific property.

public void ApplyPageBehaviors(Page page)
{
    // Apply Prism Behaviors
    base.ApplyPageBehaviors(page);
    switch(page)
    {
        case TabbedPage tabbed:
            AndroidSpecific.TabbedPage.SetIsSwipePagingEnabled(tabbed, true);
            macOSSpecific.TabbedPage.SetTabsStyle(tabbed, TabsStyle.Icons);
            break;
        case NavigationPage navPage:
            iOSSpecific.NavigationPage.SetIsNavigationBarTranslucent(navPage, true);
            macOSSpecific.NavigationPage.SetNavigationTransitionPushStyle(navPage, NavigationTransitionStyle.SlideUp);

    }
}

@dansiegel dansiegel reopened this Oct 20, 2017
brianlagunas added a commit that referenced this issue Nov 9, 2017
@brianlagunas brianlagunas added this to the Prism 7.0 milestone Nov 10, 2017
@lock
Copy link

lock bot commented Jan 29, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants