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

[iOS] Fix Frame issues #10469

Merged
merged 15 commits into from Jun 2, 2020
Merged

[iOS] Fix Frame issues #10469

merged 15 commits into from Jun 2, 2020

Conversation

jfversluis
Copy link
Member

@jfversluis jfversluis commented Apr 27, 2020

Description of Change

Overhauls the iOS Frame implementation to overcome the bugs introduced by #7518.

With this change we achieve the same thing but without the bugs and makes it more sustainable to add potential other features. Basically what happens is I add a second layer which holds the actual content so that can be clipped, etc. and the original UIView that has always been there is just responsible for the shadow.

For the implementation I have gotten some inspiration from PancakeView, so it's been tested for a while too.

Issues Resolved

API Changes

None

Platforms Affected

  • iOS

Behavioral/Visual Changes

All the bugs introduced by #7518 should be gone and the Frame should work even better than before.

However, people that based a renderer off of the previous FrameRenderer might in some (edge) cases find that something breaks because I moved the content of the Frame to a separate layer.
I think we should make the extra layer available so users can use that as well, but I think that will be something for a separate PR in vNext to not introduce new APIs on previous versions.

Before/After Screenshots

Not applicable

Testing Procedure

Go through as much Frame test cases as you can and verify things work as you'd expect. The bugs introduced mostly had to do with transforms. RotationX, TranslateY that kind of things. The previous solution introduced a separate UIView that did not take into account any of those things.

Also, pay special attention to gesture recognizers.

I've added reproductions in the gallery for almost all issues mentioned above.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@jfversluis jfversluis added t/bug 🐛 DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. p/iOS 🍎 i/regression m/high impact ⬛ ControlGallery a/frame 4.5.0 regression on 4.5.0 labels Apr 27, 2020
@jfversluis
Copy link
Member Author

Added DNM for now. I want to verify that all the linked issues are resolved by the NuGets generated from this.

protected override void Build (StackLayout stackLayout)
{
base.Build (stackLayout);

var hasShadowContainer = new StateViewContainer<Frame> (Test.Frame.HasShadow, new Frame { HasShadow = true });
var outlineColorContainer = new StateViewContainer<Frame> (Test.Frame.OutlineColor, new Frame { BorderColor = Color.Teal, });
Copy link
Member Author

Choose a reason for hiding this comment

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

API is obsolete, so removed this test case

@jfversluis jfversluis changed the title Fix [iOS] Fix Frame issues Apr 27, 2020
@samhouts samhouts added this to In Progress in v4.5.0 Apr 27, 2020
@samhouts samhouts added this to In progress in Sprint 169 Apr 27, 2020
@samhouts samhouts added the API-change Heads-up to reviewers that this PR may contain an API change label Apr 27, 2020
@jfversluis jfversluis removed the API-change Heads-up to reviewers that this PR may contain an API change label Apr 30, 2020
@jfversluis
Copy link
Member Author

Implemented a slightly better solution which allows to keep the same base class for this renderer and only adds a separate frame that will hold the actual content. Taking off the API change label.

Still want to take these NuGets and go through the "fixed issues" list to see if this works in all of these scenarios.

@samhouts samhouts moved this from Ready for Review (PRs) to Continued in next sprint in Sprint 170 May 26, 2020
@rmarinho rmarinho added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Jun 1, 2020
@rmarinho
Copy link
Member

rmarinho commented Jun 1, 2020

Failing test on iOS
VerifyTapBubbling("Frame",True)_apple_iphone_11_13_1_3

Sprint 171 automation moved this from In progress to Ready for Review (PRs) Jun 2, 2020
@rmarinho rmarinho merged commit 438940e into 4.5.0 Jun 2, 2020
v4.5.0 automation moved this from In Progress to Done Jun 2, 2020
Sprint 171 automation moved this from Ready for Review (PRs) to Done Jun 2, 2020
@rmarinho rmarinho deleted the fix-frame branch June 2, 2020 10:44
@samhouts samhouts added the a/animation 🎬 Animation stuff label Jun 2, 2020
@samhouts samhouts added this to the 4.6.0 milestone Jun 9, 2020
@samhouts samhouts added this to Done in v4.6.0 Jun 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.