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

Region Support for Xamarin.Forms #2077

Merged
merged 23 commits into from
Aug 20, 2020
Merged

Region Support for Xamarin.Forms #2077

merged 23 commits into from
Aug 20, 2020

Conversation

dansiegel
Copy link
Member

Description of Change

Yep we're closing Prism's OLDEST open issue.... Support for Regions in Xamarin.Forms!

Bugs Fixed

API Changes

Too much to list... Suffice it to say we've taken Regions from Prism.Wpf and made some slight modifications for Xamarin.Forms.

Behavioral Changes

This changes Prism.DryIoc.Wpf & Prism.Unity.Wpf to no longer register their exceptions in the Bootstrapper/PrismApplication as this will now simply be done in the ContainerExtension instead...

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard

@dansiegel dansiegel added enhancement XF DO-NOT-MERGE-!!! 🛑 Pull Request should not be merged in it's current state labels Apr 21, 2020
@dansiegel dansiegel added this to the Prism 8.0 milestone Apr 21, 2020
@dansiegel dansiegel force-pushed the wip-regions branch 4 times, most recently from 7fe506f to 90cdd78 Compare April 24, 2020 18:26
@dansiegel
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dansiegel
Copy link
Member Author

dansiegel commented Aug 12, 2020

Out of Box adapters to support:

  • CarouselView ✅
  • ContentView ✅
  • FlexLayout ✅
    • NOTE Be sure to add your Flex Basis to the Views in your Region if using Flex Layout
  • Frame ✅
  • Grid ⚠️
    • The Grid inherits from Layout like the StackLayout & FlexLayout. However the Adapter has no understanding of the Grid Layout. As such it has no ability to manage which Grid Row your Region should be added to. A custom adapter is needed to manage this.
  • RefreshView ⚠️
    • The RefreshView inherits from ContentView and as such this will technically work using the ContentViewAdapter. This however has no way of understanding the Refresh event/command/state. As such you will need a custom adapter to use the RefreshView as a region.
  • ScrollView ✅
  • StackLayout ✅
  • SwipeView ⚠️
    • The SwipeView also inherits from ContentView and technically would work with the ContentViewAdapter. However in my assessment, the Swipe view is not a good Region candidate it is the sort of control that is best used within a region due to the need to link the content to the swipe items.

Unsupported Controls:

  • CollectionView 🚫
    • known Platform bug on iOS causes hard crash
    • CollectionView is only displaying the last element despite having an ItemsSource of the ActiveViews with multiple ActiveViews. Additional work & testing will need to be done with the adapter that is part of this PR.
  • ListView 🚫
    • This control is being deprecated by the Xamarin.Forms team and is expected to be pulled out of .NET MAUI.

Development Note: The CollectionView Region Adapter is still part of this PR, however it has not been added to the RegionAdapterMappings due to the issues mentioned above. Once the issues have been resolved a follow up PR will add the support for CollectionView.

@dansiegel dansiegel marked this pull request as ready for review August 18, 2020 22:26
@dansiegel dansiegel removed the DO-NOT-MERGE-!!! 🛑 Pull Request should not be merged in it's current state label Aug 19, 2020
/// </summary>
/// <typeparam name="TView">The Type of <see cref="View"/> to register</typeparam>
/// <param name="containerRegistry"><see cref="IContainerRegistry"/> used to register type for Navigation.</param>
/// <param name="name">The unique name to register with the Page</param>
Copy link
Member

Choose a reason for hiding this comment

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

the view?

/// <typeparam name="TView">The Type of <see cref="View"/> to register</typeparam>
/// <param name="containerRegistry"><see cref="IContainerRegistry"/> used to register type for Navigation.</param>
/// <param name="name">The unique name to register with the Page</param>
public static void RegisterForNavigation<TView>(this IContainerRegistry containerRegistry, string name = null)
Copy link
Member

Choose a reason for hiding this comment

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

rename to RegisterForRegionNavigation

/// </summary>
/// <returns>A new instance of <see cref="AllActiveRegion"/>.</returns>
protected override IRegion CreateRegion() =>
new AllActiveRegion();
Copy link
Member

Choose a reason for hiding this comment

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

new region()?

throw new InvalidOperationException(Resources.LayoutViewHasChildrenException);
}

BindableLayout.SetItemsSource(regionTarget, region.ActiveViews);
Copy link
Member

Choose a reason for hiding this comment

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

should this be views?

/// </summary>
/// <returns>A new instance of <see cref="AllActiveRegion"/>.</returns>
protected override IRegion CreateRegion() =>
new AllActiveRegion();
Copy link
Member

Choose a reason for hiding this comment

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

region?

/// </summary>
/// <returns>A new instance of <see cref="SingleActiveRegion"/>.</returns>
protected override IRegion CreateRegion() =>
new SingleActiveRegion();
Copy link
Member

Choose a reason for hiding this comment

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

just region

SynchronizeRegionContext();

// Now register for events to keep them in sync
HostControlRegionContext.PropertyChanged += RegionContextObservableObject_PropertyChanged;
Copy link
Member

Choose a reason for hiding this comment

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

let's look into this for memory leaks

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants