codeandtheory / ybottomsheet-ios Goto Github PK

Intro

Y—CoreUI 1.6.0+ has a new Animation struct (used in Y—Snackbar) that lets us configure component animations without even needing to know the animation type (e.g. regular curve vs spring-dampening). We should adopt it for Y—BottomSheet's animations to allow increased customizations.

Task

1. Replace the following three properties on BottomSheetController.Appearance

public var animationDuration: TimeInterval
public var presentAnimationCurve: UIView.AnimationOptions
public var dismissAnimationCurve: UIView.AnimationOptions

with two new properties:

public var presentAnimation: Animation
public var dismissAnimation: Animation

2. Add an Animation+BottomSheet.swift file with the following default animations defined:

/// Default animation properties for bottom sheet
public extension Animation {
    /// Default animation for presenting bottom sheet (ease in)
    static let defaultPresent = Animation(curve: .regular(options: .curveEaseIn))

    /// Default animation for dismissing bottom sheet (ease out)
    static let defaultDismiss= Animation(curve: .regular(options: .curveEaseOut))
}

3. In BottomSheetController.Appearance use the above defaults as the default parameters when initializing an Appearance object.

4. Have the present and dismiss animators leverage these new appearance properties for their animations.

5. Update unit tests for 100% coverage and properly testing the new functionality.

Note: when Reduced Motion is on, we will ignore the provided curve (it might be spring-dampening which does not make sense for a cross-dissolve) and replace it with a default easing curve (see Y—Snackbar).

Replace these two private properties on BottomSheetController:

    private let minimumTopOffset: CGFloat = 44
    private let minimumContentHeight: CGFloat = 88

with two public properties on BottomSheetController.Appearance.Layout (same default values).

Expand unit tests as needed to maintain 100% code coverage.

In the child view controller variant only, the corner radius is not being applied.
(Or rather it is being applied, but it is not cropping the child controller content).

For now setting childController.view.backgroundColor = .clear fixes the issue, but we need a better solution.

@SahilSainiYML found a typo in the documentation comments for BottomSheetController.Appearance (double period)

This needs to be fixed in the code file and also in the README where it was repeated.

• Add shields.io badges to README for
  • swift version
  • platform compatibility
• In the above order
• Immediately below the banner image but above the italicized tagline

Intro

SwiftLint 0.51.0 now requires us to manually re-enable all rules disabled on the file level.

Task

1. Add corresponding // swiftlint: enable rule_name comments at the end of all files that contain a disable entry (these are mostly test files)
2. Update any instances of swiftlint:enable or swiftlint:disable with a version that has a single space immediately following the colon.

Make sure you're running the latest SwiftLint by running

brew install swiftlint

and test locally to make sure everything is fine.

Insert the following in between Usage and Installation sections:

Dependencies
----------

Y—BottomSheet depends upon our [Y—CoreUI](https://github.com/yml-org/ycoreui) and [Y—MatterType](https://github.com/yml-org/ymattertype) frameworks (both also open source and Apache 2.0 licensed).

The Challenge

We recently added the LayoutSizable protocol so that both views and view controllers can report their layout size to the bottom sheet controller and the sheet can be the correct height when presented.

But what happens if that view (or view controller) changes size? e.g. new data arrives and the table view now has 6 rows instead of 4? How can the table view let the sheet know that it should recalculate its ideal size?

A Suggestion

1. Add an overridable method invalidatesLayoutSize() to UIViewController. A table view controller could call this when its data changes.
2. The default implementation of this method on UIViewController is to call the same method on its parent, thus moving it up the view controller chain until it finds a suitable responder (or reaches the top-level view controller).
3. BottomSheetController override this method and instead of calling to its parent, it just calls updateChildView() which will then adjust the idealContentHeightAnchor by re-evaluating layoutSize on the child.

Intro

TheUIViewController override of initializing a bottom sheet is much more problematic than the UIView override when it comes to the sheet being able to determine the correct intrinsic content size. Ideally size should be driven by the content: larger content leads to a larger bottom sheet (up to the max, naturally).

This works well when passing in a UIView to display because views generally know their own intrinsicContentSize (which can be easily overridden in the UIView subclass).

It also works ok when passing UIViewController subclasses. If the view is properly configured, it tends to return its correct content size (the exception being if it has a UIScrollView subview).

However, system view controller subclasses such as UINavigationController and UITableViewController (the latter because it is a scroll view) don't return the correct intrinsic content size (and their views cannot be easily overridden to return the correct content size) which leads to bottom sheets that are not the correct size.

Because we can't easily modify these system classes to return the desired intrinsicSize, we should provide another mechanism so that these view controllers can report their size.

Task

1. Clean up sheet sizing: move properties to layout. We should be able to specify minimum, ideal, and maximum size of the sheet.
2. Provide a mechanism whereby navigation controllers report the correct ideal size
3. Provide a mechanism whereby table view controllers report the correct ideal size
4. Fix bug where table view sheets do not clip their corners

When enabled, our present/dismiss animators should fade the sheet in and out instead of sliding it on/off the bottom of the screen.

So instead of changing the sheet frame, we would change the sheet alpha (setting both the before and after values).

There is currently a gap above the header (or navigation bar when UINavigationController is used) that is equal in height to the sheet's corner radius. This space is used to house the drag indicator view, but serves no purpose when it is hidden, so we should not waste the vertical screen space in that case.

• Hide the top portion that houses the indicator bar when indicatorAppearance == nil, but retain the sheet's corner radius.

Hide the Pull Request Template file by moving it to the .github directory.

Rename PULL_REQUEST_TEMPLATE.md to .github/pull_request_template.md
Update link in README under the ### Pull Requests section to reflect the new name and location.

Two changes requested:

1. Expose a new public method to call updateViewAppearance() internally to update to new layout size.

Today we are resetting .appearance property which is calling updateViewAppearance().

Something like

public func updateViews() {
   updateViewAppearance()
}

2. When there is a new height reported for the LayoutSizable protocol from the view controller, animate to the new height. Today the resize is happening but it just jumps. It's not a nice animation like we have how it opens the bottom sheet for the first time.

See how we used an enum (and unit test) in YTags for the default close button image, and do something similar, except:

1. Update package to require latest YCoreUI 1.7.0+
2. add a new Images enum with a single case xmark (see YTags)
3. conform Images enum to SystemImage
4. set renderingMode to .alwaysTemplate
5. test with size scaling. If problematic, set textStyle to nil (no scaling)
6. add static defaultImage: UIImage property that equals Images.xmark.image (see YTags)
7. use .defaultImage as the default parameter for the close button instead of UIImage(systemName: "xmark"). Update documentation comments and README accordingly if necessary.

Intro

In Voice Over mode we should make the transparent area above the sheet a button to enable it to be explicitly tapped.
We basically already have this ability in non-VO mode (tap anywhere on dimmer to dismiss), we just need to enable it as an explicit button when VO is enabled.

Task

• Add a child view that will be pinned to top and sides of BottomSheetController.view and have its bottom pinned to the top of sheetView. This will give it a frame of the space above the sheet (and will change as user resizes or the content grows).
• Important: None of this should apply when appearance.isDismissAllowed == false.
• We should probably move the tap gesture recognizer from the dimmer to this view
• It should have the .button accessibility trait
• It should have accessibilityLabel = "Dismiss" (same string as the close button)
• It should have no accessibilityHint
• It should have an accessibilityIdentifier = sheet.dimmer.close
• (We should add an identifier to our default close button of sheet.button.close):

/// Accessibility Identifiers
enum AccessibilityIdentifiers {
    /// Button ID
    static let buttonId = "sheet.button.close"
    /// Dimmer ID
    static let dimmerId = "sheet.dimmer.close"
}

Intro

Some users may want to force user to take an action (e.g. press a Button) on the bottom sheet's child and not want the user to be able to dismiss the sheet. While they can hide the close button via the appearance, they have no way of disabling the swipe down, tap on dimmer, or accessibility escape gesture ways of dismissing the sheet.

Task

• Add a Boolean flag to the bottom sheet appearance allowDismiss, defaults = true and make sure the swipe down, pan down, tap on dismiss, and escape gesture all don't do anything when allowDismiss == false.
• We only need a single flag to control all these methods of dismissing.
• allowDismiss should not affect the close button. If that is visible, then tapping on it should always dismiss the sheet.
• allowDismiss should not affect programmatically calling dismiss(). Otherwise user would never be able to dismiss the sheet even if they take whatever action the child view requires.
• Add unit tests to confirm that when allowDismiss is set to false, the various dismissal methods above do not work.