Mhd/194 floating panel

[mhdostal]

This is the FloatingPanel implementation for DataCollection v1.3. It also moves the Bookmarks from their own popover to a floating panel (LayerContents and Identify results will move later).

In addition to a code-level review, I'd be interested to hear how the floating panel "feels" when resizing and switching between visible states (minimized, partial, full).

[mhdostal]

cc: @philium @pgruenler @rolson @nCastle1 @puneet-pdx @njarecha @sbaskaran - if you have a chance, please review. I need at least one more reviewer.

[nCastle1]

I've taken a look and have a few comments on the behavior of the bottom sheet:

• When showing just the title/preview for the bottom sheet, I think the content of the sheet should be hidden. Currently on iPhone XR, the bookmark list is visible behind the handlebar/in the unsafe area when in portrait. Apple maps will hide the sheet content even when it would otherwise be visible behind the handlebar.
• When the bottom sheet is expanded and the user pulls down on the bookmark list, the bottom sheet doesn't start transitioning to the intermediate state. Apple maps will start panning the sheet once the list can't scroll anymore.
• Noticed the bottom sheet currently fades out. I wonder if it would be more natural if it slide in from the bottom when shown at the bottom, and from the left when shown on the side?

[esreli]

Quick note, please rebase this PR onto the new branch v.next-1.3.

[mhdostal]

Quick note, please rebase this PR onto the new branch v.next-1.3.

@esreli Done.

[mhdostal]

@esreli @philium - I've made the changes you recommended. I've made some pretty significant changes to the architecture, including:

• rename main classes to change ViewController to Controller, similar to how subclasses of UIViewController like UINavigationController.
• rethought the FloatingPanelItem, now it is it's own class (not a protocol) and will be available to any UIViewController, which implements FloatingPanelEmbeddable. Again, this is similar to UINavigationController.navigationItem.
• rethought the method behind the header display; now it is it's own UINavigationController whose push/pops are controlled by the main content navigation controller (aptly titled contentViewController). Added custom animation to both view controllers, via a custom subclass of UINavigationController: FloatingPanelNavigationController.
• Added a static func instantiate(_ initialViewController: FloatingPanelEmbeddable) -> FloatingPanelController to simplify creating a floating panel and setting the initial VC.

I may have missed some small bits from the original reviews, so please re-raise those issues if necessary.

[mhdostal]

@sbaskaran - Can you please review this along with Phil and Eli? Thanks.

[mhdostal]

As of iOS 13 UIButton supports the .close button type, which in effect looks like the close button you've introduced (x-close). Would you provide conditional support for devices running iOS 12 to use your (x-close) button and iOS 13 and on to use the close button supplied by UIKit?

@esreli - the buttonType property on UIButton is readonly, so I have to create the button with that type, either in the Storyboard or programmatically. Is there any mechanism to have such conditional-construction in the Storyboard?

[esreli]

https://github.com/Esri/data-collection-ios/pull/247#issuecomment-699097074

@mhdostal, I don't believe there is. This would probably require building the UIButton programmatically.

[mhdostal]

#247 (comment)

@mhdostal, I don't believe there is. This would probably require building the UIButton programmatically.

Ok. I'm going to leave it as-is then and we can re-look at it when the SDK moves to iOS 13 as a minimum.

[mhdostal]

@esreli, @philium , @sbaskaran - I've address all the feedback, please take another look. Thanks!

[mhdostal]

@philium I made the requested changes, ready for re-review. Thanks!

[rolson]

👍