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

[iOS] Fixes for iOS11 #1238

Merged
merged 20 commits into from Nov 2, 2017

Conversation

Projects
None yet
9 participants
@rmarinho
Member

rmarinho commented Oct 27, 2017

Description of Change

Fixes and new features for iOS11 support.

Platform specifics for SafeArea, SafeAreaInsets and LargeTitles.

Bugs Fixed

API Changes

Added:
Page iOS Platform Specific

public enum LargeTitleDisplayMode
{
	Automatic,
	Always,
	Never
}


public static readonly BindableProperty LargeTitleDisplayProperty = BindableProperty.Create(nameof(LargeTitleDisplay), typeof(LargeTitleDisplayMode), typeof(Page), LargeTitleDisplayMode.Automatic);

public static LargeTitleDisplayMode GetLargeTitleDisplay(BindableObject element)
public static void SetLargeTitleDisplay(BindableObject element, LargeTitleDisplayMode value)
public static IPlatformElementConfiguration<iOS, FormsElement> SetLargeTitleDisplay(this IPlatformElementConfiguration<iOS, FormsElement> config, LargeTitleDisplayMode value)
public static LargeTitleDisplayMode LargeTitleDisplay(this IPlatformElementConfiguration<iOS, FormsElement> config)
public static bool UsingSafeArea(this IPlatformElementConfiguration<iOS, FormsElement> config)



static readonly BindablePropertyKey SafeAreaInsetsPropertyKey = BindableProperty.CreateReadOnly(nameof(SafeAreaInsets), typeof(Thickness), typeof(Page), default(Thickness))

public static readonly BindableProperty SafeAreaInsetsProperty = SafeAreaInsetsPropertyKey.BindableProperty;

public static Thickness GetSafeAreaInsets(BindableObject element)
public static Thickness SafeAreaInsets(this IPlatformElementConfiguration<iOS, FormsElement> config)
public static IPlatformElementConfiguration<iOS, FormsElement> SetSafeAreaInsets(this IPlatformElementConfiguration<iOS, FormsElement> config, Thickness value)

NavigationPage iOS Platform Specific

public static readonly BindableProperty UseLargeTitlesProperty = BindableProperty.Create("UseLargeTitles", typeof(bool), typeof(Page), false);

public static bool GetPrefersLargeTitles(BindableObject element)
public static void SetPrefersLargeTitles(BindableObject element, bool value)
public static IPlatformElementConfiguration<iOS, FormsElement> SetPrefersLargeTitles(this IPlatformElementConfiguration<iOS, FormsElement> config, bool value)
public static bool PrefersLargeTitles(this IPlatformElementConfiguration<iOS, FormsElement> config)

CellRenderer

public virtual void SetBackgroundColor(UITableViewCell tableViewCell, Cell cell, UIColor color)

Behavioral Changes

CellView will extend the view background color to the UITableViewCell, this way it will look good when we Layout the ContentView frame with insets.

PR Checklist

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

Listview with ViewCell with NO Safe area
simulator screen shot - iphone x - 2017-11-02 at 15 28 04

Listview with ViewCell with Safe area
simulator screen shot - iphone x - 2017-11-02 at 15 27 54

CarouselPage with NO Safe area
simulator screen shot - iphone x - 2017-11-02 at 15 27 39

CarouselPage with Safe area
simulator screen shot - iphone x - 2017-11-02 at 15 27 21

TabbedPage with NO Safe area
simulator screen shot - iphone x - 2017-11-02 at 15 27 12

TabbedPage with Safe area
simulator screen shot - iphone x - 2017-11-02 at 15 27 02

NavigationPage with NO Safe area
simulator screen shot - iphone x - 2017-11-02 at 15 25 51

NavigationPage with Safe area
simulator screen shot - iphone x - 2017-11-02 at 15 25 42

ContentPage with NO Safe area
simulator screen shot - iphone x - 2017-11-02 at 15 25 32

ContentPage with Safe area
simulator screen shot - iphone x - 2017-11-02 at 15 25 24

ListView with Grouping safe area
simulator screen shot - iphone x - 2017-11-02 at 15 36 42

landscape
simulator screen shot - iphone x - 2017-11-02 at 15 37 07

ListView with Grouping NO safe area (see group header scrolling up)
simulator screen shot - iphone x - 2017-11-02 at 15 36 15

landscape
simulator screen shot - iphone x - 2017-11-02 at 15 37 47

TableView with Safe Area
simulator screen shot - iphone x - 2017-11-02 at 15 37 58

landscape
simulator screen shot - iphone x - 2017-11-02 at 15 38 00

TableView with no Safe Area (we do set the background color to viewcell view but layout the cell in the right place)
simulator screen shot - iphone x - 2017-11-02 at 15 38 10

landscape
simulator screen shot - iphone x - 2017-11-02 at 15 38 15

@StephaneDelcroix

I'm quite sure this does the right thing, but I challenged the public API for safe area.

I'm sure it's coming, but at this point, it misses test cases and some screenshots would looks nice too

}
public static readonly BindableProperty LargeTitleDisplayProperty = BindableProperty.Create(nameof(LargeTitleDisplay), typeof(LargeTitleDisplayMode), typeof(Page), LargeTitleDisplayMode.Automatic);

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Oct 30, 2017

Member

is there an example of this being used ?

@StephaneDelcroix

StephaneDelcroix Oct 30, 2017

Member

is there an example of this being used ?

This comment has been minimized.

@rmarinho

rmarinho Nov 1, 2017

Member

Yes we have gallery now

@rmarinho

rmarinho Nov 1, 2017

Member

Yes we have gallery now

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Nov 1, 2017

Member

what's the difference between UseLargeTitlesProperty and LargeTitleDisplayProperty ? can't those be combined ?

@StephaneDelcroix

StephaneDelcroix Nov 1, 2017

Member

what's the difference between UseLargeTitlesProperty and LargeTitleDisplayProperty ? can't those be combined ?

This comment has been minimized.

@rmarinho

rmarinho Nov 1, 2017

Member

I think we should reflect the iOS api as this is a PlatformSpecific, easy for those o google for info.

@rmarinho

rmarinho Nov 1, 2017

Member

I think we should reflect the iOS api as this is a PlatformSpecific, easy for those o google for info.

Show outdated Hide outdated Xamarin.Forms.Core/PlatformConfiguration/iOSSpecific/Page.cs
Show outdated Hide outdated Xamarin.Forms.Core/PlatformConfiguration/iOSSpecific/Page.cs
case LargeTitleDisplayMode.Always:
NavigationItem.LargeTitleDisplayMode = UINavigationItemLargeTitleDisplayMode.Always;
break;
case LargeTitleDisplayMode.Automatic:

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Oct 30, 2017

Member

future proof your code by adding a default: here

@StephaneDelcroix

StephaneDelcroix Oct 30, 2017

Member

future proof your code by adding a default: here

This comment has been minimized.

@rmarinho

rmarinho Nov 1, 2017

Member

I wanted to but c#7 was giving me errors?! going to look in the morning

@rmarinho

rmarinho Nov 1, 2017

Member

I wanted to but c#7 was giving me errors?! going to look in the morning

Show outdated Hide outdated Xamarin.Forms.Core/PlatformConfiguration/iOSSpecific/Page.cs
@davidortinau

This comment has been minimized.

Show comment
Hide comment
@davidortinau
Contributor

davidortinau commented Oct 30, 2017

@StephaneDelcroix

public API needs to be discussed. implementation looks ok

}
public static readonly BindableProperty LargeTitleDisplayProperty = BindableProperty.Create(nameof(LargeTitleDisplay), typeof(LargeTitleDisplayMode), typeof(Page), LargeTitleDisplayMode.Automatic);

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Nov 1, 2017

Member

what's the difference between UseLargeTitlesProperty and LargeTitleDisplayProperty ? can't those be combined ?

@StephaneDelcroix

StephaneDelcroix Nov 1, 2017

Member

what's the difference between UseLargeTitlesProperty and LargeTitleDisplayProperty ? can't those be combined ?

@@ -56,5 +56,91 @@ public static UIStatusBarAnimation PreferredStatusBarUpdateAnimation(this IPlatf
SetPreferredStatusBarUpdateAnimation(config.Element, value);
return config;
}
public static readonly BindableProperty UseSafeAreaProperty = BindableProperty.Create("UseSafeArea", typeof(bool), typeof(Page), false, propertyChanged: (bindable, oldValue, newValue) =>

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Nov 1, 2017

Member

my original comment still stand. I don't think this should be exposed

@StephaneDelcroix

StephaneDelcroix Nov 1, 2017

Member

my original comment still stand. I don't think this should be exposed

This comment has been minimized.

@rmarinho

rmarinho Nov 1, 2017

Member

How will the user opt out ? I have a doubt on make the default to true

@rmarinho

rmarinho Nov 1, 2017

Member

How will the user opt out ? I have a doubt on make the default to true

@rmarinho

This comment has been minimized.

Show comment
Hide comment
@rmarinho

rmarinho Nov 2, 2017

Member

Updated with screenshots

Member

rmarinho commented Nov 2, 2017

Updated with screenshots

@jassmith jassmith merged commit 4d49e57 into master Nov 2, 2017

10 of 12 checks passed

VSTS: Android API23 Validation Fast Renderers UITests Finished
Details
VSTS: Android API25 Validation Legacy Renderers UITests Finished
Details
VSTS: Android API19 Validation Fast Renderers UITests Finished
Details
VSTS: Android API19 Validation Legacy Renderers UITests Finished
Details
VSTS: Android API23 Validation Legacy Renderers UITests Finished
Details
VSTS: Android API25 Validation Fast Renderers UITests Finished
Details
VSTS: Xamarin Forms OSX pull-1238 - (1103504) Succeeded
Details
VSTS: Xamarin Forms Windows 1103504 Succeeded
Details
VSTS: Xamarin Forms Windows (PR Builds) Started PR process $(TriggeredBuildIds)
Details
VSTS: iOS10 Validation UITests Finished
Details
VSTS: iOS11 Validation UITests Finished
Details
VSTS: iOS9 Validation UITests Finished
Details

jassmith added a commit that referenced this pull request Nov 2, 2017

[iOS] Fixes for iOS11 (#1238)
* [iOS] Add SafeArea support

* [iOS] Only apply SafeArea on iOS11 or newer

* [iOS] Handle SafeAreaInsets on ViewCell and GroupCell

* [iOS] Set page padding from safe area

* [Controls] Use SafeArea PS

* [iOS] Add platform specific for LargeTiles

* [iOS] Add LargeTitleDisplayMode platform specific for ios11

* [iOS] Fix page size when large title collapses

* [iOS] Add platform specific to expose SafeAreaInsets from iOS11

* [Controls] Large titles iOS specific gallery page

* [iOS] Remove comment code on PS example

* [Controls] Add gallery sample for Safe Area PS

* [iOS] Inside a TabbedPage safe area bottom is handle by UITabViewController

* [Core,iOS]If we are not using safearea set the padding to the default

* [iOS] Revert SafeAreas on navpage

* [iOS] Fix safe area inset for ViewCell

* [iOS] Handle ViewCell margin correctly on iOS11

* [Core,iOS] Rename to UsingSafeArea, use BPKey instead for SafeAreaInsets

* [Core,iOS] Rename to UsingLargeTitles

* [iOS,Core] Rename to PrefersLargeTitles
@@ -84,6 +94,12 @@ public override void LayoutSubviews()
var contentFrame = ContentView.Frame;
var view = ViewCell.View;
if (Forms.IsiOS11OrNewer)
{

This comment has been minimized.

@bentmar

bentmar Nov 9, 2017

Contributor

This should really be up to the developer to decide. This will break every alignments to content above/below listview when on ios11. This gap will also show the color of the Elements backgroundcolor that the listview is layed out on.

link to my post in forum with more details https://forums.xamarin.com/discussion/comment/306004/#Comment_306004

Im not sure, but I think this is what causes the gap on viewcells

@bentmar

bentmar Nov 9, 2017

Contributor

This should really be up to the developer to decide. This will break every alignments to content above/below listview when on ios11. This gap will also show the color of the Elements backgroundcolor that the listview is layed out on.

link to my post in forum with more details https://forums.xamarin.com/discussion/comment/306004/#Comment_306004

Im not sure, but I think this is what causes the gap on viewcells

This comment has been minimized.

@rmarinho

rmarinho Nov 9, 2017

Member

please open a bug so we can investigate, i understand your point.

@rmarinho

rmarinho Nov 9, 2017

Member

please open a bug so we can investigate, i understand your point.

This comment has been minimized.

@dhaligas

dhaligas Nov 13, 2017

@rmarinho @bentmar I can confirm that all of my ViewCells are shifted over on the left in iOS 11 with this release. This occurs on all types of simulators. Not sure why there would be left padding as a default. This should only occur on landscape on a X to avoid the notch.

This is a breaking change

@dhaligas

dhaligas Nov 13, 2017

@rmarinho @bentmar I can confirm that all of my ViewCells are shifted over on the left in iOS 11 with this release. This occurs on all types of simulators. Not sure why there would be left padding as a default. This should only occur on landscape on a X to avoid the notch.

This is a breaking change

@rmarinho rmarinho referenced this pull request Nov 14, 2017

Merged

[iOS] Revert iOS11 safe area fix on ViewCell #1270

0 of 4 tasks complete

@rmarinho rmarinho deleted the fix-ios11safearea branch Dec 4, 2017

@samhouts samhouts added this to the 2.5.0 milestone May 5, 2018

@samhouts samhouts modified the milestones: 2.5.0, 2.4.0 Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment