Scroll indicators update - positioning correction, support for both indicators at the same time

Sources/UIScrollView+indicatorsInternal.swift

@@ -6,26 +6,52 @@
 //  Copyright © 2018 flowkey. All rights reserved.
 //
 
-// NOTE: Everything in this file must be private / internal-only.
+// NOTE: Everything in this file must be private or internal.
 // You can't override methods that were defined in an extension.
 
-internal extension UIScrollView {
-    static let indicatorDistanceFromScrollViewFrame: CGFloat = 2.5
+extension UIScrollView {
+    private var indicatorLengths: (horizontal: CGFloat, vertical: CGFloat) {
+        let minIndicatorLength: CGFloat = 30.0
+        return (
+            horizontal: max(minIndicatorLength, (bounds.width / contentSize.width) * bounds.width),
+            vertical: max(minIndicatorLength, (bounds.height / contentSize.height) * bounds.height)
+        )
+    }
+
+    private var indicatorOffsetsInContentSpace: (horizontal: CGFloat, vertical: CGFloat) {
+        let indicatorDistanceFromScrollViewFrame: CGFloat = 2.5
 
-    func indicatorOffsetsInContentSpace() -> (horizontal: CGFloat, vertical: CGFloat) {
         let totalContentArea = (
             horizontal: contentInset.left + contentSize.width + contentInset.right,
             vertical: contentInset.top + contentSize.height + contentInset.bottom
         )
 
         let scrollViewProgress = (
-            horizontal: (contentInset.left + contentOffset.x) / (totalContentArea.horizontal + bounds.width),
-            vertical: (contentInset.top + contentOffset.y) / (totalContentArea.vertical + bounds.height)
+            horizontal: (contentInset.left + contentOffset.x) / (totalContentArea.horizontal - bounds.width),
+            vertical: (contentInset.top + contentOffset.y) / (totalContentArea.vertical - bounds.height)
+        )
+
+        let shouldShowBothIndicators = shouldShowHorizontalScrollIndicator && shouldShowVerticalScrollIndicator
+
+        // These values are based on iOS
+        let additionalSpacingToPreventOverlap = (
+            horizontal: shouldShowBothIndicators ? 2 * indicatorDistanceFromScrollViewFrame : 0,
+            vertical: shouldShowBothIndicators ? indicatorDistanceFromScrollViewFrame : 0
+        )
+
+        let totalSpacingFromFrameSides = (
+            horizontal: totalScrollIndicatorInsets.left + totalScrollIndicatorInsets.right + additionalSpacingToPreventOverlap.horizontal,
+            vertical: totalScrollIndicatorInsets.bottom + totalScrollIndicatorInsets.top + additionalSpacingToPreventOverlap.vertical
+        )
+
+        let lengthOfAvailableSpaceForIndicators = (
+            horizontal: bounds.width - (indicatorLengths.horizontal + totalSpacingFromFrameSides.horizontal),
+            vertical: bounds.height - (indicatorLengths.vertical + totalSpacingFromFrameSides.vertical)
         )
 
         let indicatorOffsetInBounds = (
-            horizontal: scrollViewProgress.horizontal * bounds.size.width,
-            vertical: scrollViewProgress.vertical * bounds.size.height
+            horizontal: scrollViewProgress.horizontal * lengthOfAvailableSpaceForIndicators.horizontal,
+            vertical: scrollViewProgress.vertical * lengthOfAvailableSpaceForIndicators.vertical
         )
 
         return (
@@ -34,54 +60,52 @@ internal extension UIScrollView {
         )
     }
 
-    var shouldLayoutHorizontalScrollIndicator: Bool {
+    private var shouldShowHorizontalScrollIndicator: Bool {
         return showsHorizontalScrollIndicator && contentSize.width > bounds.width
     }
 
-    var shouldLayoutVerticalScrollIndicator: Bool {
+    private var shouldShowVerticalScrollIndicator: Bool {
         return showsVerticalScrollIndicator && contentSize.height > bounds.height
     }
 
+    internal func layoutScrollIndicatorsIfNeeded() {
+        guard
+            contentSize.width > frame.size.width,
+            contentSize.height > frame.size.height
+        else { return }
 
-    func layoutScrollIndicatorsIfNeeded() {
-        guard shouldLayoutHorizontalScrollIndicator || shouldLayoutVerticalScrollIndicator else { return }
-
-        let indicatorDistanceFromScrollViewFrame = UIScrollView.indicatorDistanceFromScrollViewFrame
-        let indicatorOffsets = indicatorOffsetsInContentSpace()
-        let indicatorLengths = (horizontal: (bounds.width / contentSize.width) * bounds.width,
-                                vertical: (bounds.height / contentSize.height) * bounds.height)
+        let distanceFromFrame = (
+            horizontal: indicatorThickness + totalScrollIndicatorInsets.bottom,
+            vertical: indicatorThickness + totalScrollIndicatorInsets.right
+        )
 
-        if shouldLayoutHorizontalScrollIndicator {
-            horizontalScrollIndicator.frame = CGRect(
-                x: indicatorDistanceFromScrollViewFrame + indicatorOffsets.horizontal,
-                y: bounds.height - (indicatorThickness + indicatorDistanceFromScrollViewFrame),
-                width: indicatorLengths.horizontal,
-                height: indicatorThickness
-            )
-        }
+        horizontalScrollIndicator.frame = CGRect(
+            x: totalScrollIndicatorInsets.left + indicatorOffsetsInContentSpace.horizontal,
+            y: bounds.height + contentOffset.y - distanceFromFrame.horizontal,
+            width: indicatorLengths.horizontal,
+            height: indicatorThickness
+        )
 
-        if shouldLayoutVerticalScrollIndicator {
-            verticalScrollIndicator.frame = CGRect(
-                x: bounds.width - (indicatorThickness + indicatorDistanceFromScrollViewFrame),
-                y: indicatorDistanceFromScrollViewFrame + indicatorOffsets.vertical,
-                width: indicatorThickness,
-                height: indicatorLengths.vertical
-            )
-        }
+        verticalScrollIndicator.frame = CGRect(
+            x: bounds.width + contentOffset.x - distanceFromFrame.vertical,
+            y: totalScrollIndicatorInsets.top + indicatorOffsetsInContentSpace.vertical,
+            width: indicatorThickness,
+            height: indicatorLengths.vertical
+        )
     }
 
     // On iOS this seems to occur with no animation at all:
-    func showScrollIndicators() {
-        if shouldLayoutHorizontalScrollIndicator {
+    internal func showScrollIndicators() {
+        if shouldShowHorizontalScrollIndicator {
             horizontalScrollIndicator.alpha = 1
         }
 
-        if shouldLayoutVerticalScrollIndicator {
+        if shouldShowVerticalScrollIndicator {
             verticalScrollIndicator.alpha = 1
         }
     }
 
-    func hideScrollIndicators() {
+    internal func hideScrollIndicators() {
         UIView.animate(
             withDuration: 0.25,  // these values have been hand-tuned
             delay: 0.05,         // to match iOS

Sources/UIScrollView.swift

@@ -56,12 +56,6 @@ open class UIScrollView: UIView {
         )
     }
 
-    public var indicatorStyle: UIScrollViewIndicatorStyle = .`default` {
-        didSet { applyScrollIndicatorsStyle() }
-    }
-
-    // TODO: var scrollIndicatorInsets
-
     var weightedAverageVelocity: CGPoint = .zero
 
     override public init(frame: CGRect) {
@@ -78,6 +72,39 @@ open class UIScrollView: UIView {
         }
     }
 
+    // When adding subviews, make sure that the scroll indicators stay on top.
+    // Subview list goes back-to-front, so indicators need to take the last two positions.
+    // We use the earliestIndicatorInSubviewHierarchy variable to make sure new views are always inserted below them.
+    private lazy var firstIndicatorInSubviews: UIView? = {
+        return subviews.first(where: { $0 === horizontalScrollIndicator || $0 === verticalScrollIndicator })
+    }()
+
+
+    open override func addSubview(_ view: UIView) {
+        // Indicators should always stay on top of the view hierarchy (so the end of the subviews list, at two highest indexes)
+        // we add new views "below them" (so one position before the scroll indicator with the lower of two highest indexes)
+        if
+            view !== horizontalScrollIndicator,
+            view !== verticalScrollIndicator,
+            let firstIndicatorInSubviews = firstIndicatorInSubviews
+        {
+            super.insertSubview(view, belowSubview: firstIndicatorInSubviews)
+        } else {
+            super.addSubview(view)
+        }
+    }
+
+    open override func insertSubview(_ view: UIView, at index: Int) {
+        // Indicators should always stay on top of the view hierarchy (so the end of the subviews list, at two highest indexes)
+        // if we are asked to insert at their positions, we insert at the first non-indicator index instead
+        if let firstIndicatorInSubviews = firstIndicatorInSubviews {
+            let indexOfFirstIndicator = subviews.index(of: firstIndicatorInSubviews)!
+            super.insertSubview(view, at: min(index, indexOfFirstIndicator))
+        } else {
+            super.insertSubview(view, at: index)
+        }
+    }
+
     open var isDecelerating = false {
         didSet {
             // Hide when we stop decelerating, but only when that wasn't because of a pan
@@ -120,11 +147,27 @@ open class UIScrollView: UIView {
     open var contentInset: UIEdgeInsets = .zero
     open var contentSize: CGSize = .zero
 
-
     // MARK: Scroll Indicators
 
+    // Matching Apple's value
     let indicatorThickness: CGFloat = 2.5
 
+    // Determined experimentally to be as close as possible to Apple's (likely exactly matching)
+    private let baseScrollIndicatorInsets = UIEdgeInsets(top: 2.5, left: 2.5, bottom: 2.5, right: 2.5)
+
+    public var indicatorStyle: IndicatorStyle = .`default` {
+        didSet { applyScrollIndicatorsStyle() } // also called on `init` separately
+    }
+
+    public var scrollIndicatorInsets = UIEdgeInsets.zero
+
+    var totalScrollIndicatorInsets: UIEdgeInsets {
+        return UIEdgeInsets(top: baseScrollIndicatorInsets.top + scrollIndicatorInsets.top,
+                            left: baseScrollIndicatorInsets.left + scrollIndicatorInsets.left,
+                            bottom: baseScrollIndicatorInsets.bottom + scrollIndicatorInsets.bottom,
+                            right: baseScrollIndicatorInsets.right + scrollIndicatorInsets.right)
+    }
+
     private func applyScrollIndicatorsStyle() {
         for scrollIndicator in [verticalScrollIndicator, horizontalScrollIndicator] {
             scrollIndicator.layer.cornerRadius = indicatorThickness / 2
@@ -147,19 +190,21 @@ public protocol UIScrollViewDelegate: class {
     func scrollViewDidEndDragging(_ scrollView: UIScrollView, willDecelerate: Bool)
 }
 
-public enum UIScrollViewIndicatorStyle {
-    case `default`
-    case black
-    case white
-
-    var backgroundColor: UIColor {
-        switch self {
-        // Default according to iOS UIKit docs is "black with a white border".
-        // But actually it's a black stretchable image with a peak opacity of 0.35.
-        // We render it differently, so we add a little opacity to get a similar effect:
-        case .`default`: return UIColor.black.withAlphaComponent(0.37)
-        case .black: return UIColor.black
-        case .white: return UIColor.white
+extension UIScrollView {
+    public enum IndicatorStyle {
+        case `default`
+        case black
+        case white
+
+        var backgroundColor: UIColor {
+            switch self {
+            // Default according to iOS UIKit docs is "black with a white border".
+            // But actually it's a black stretchable image with a peak opacity of 0.35.
+            // We render it differently, so we add a little opacity to get a similar effect:
+            case .`default`: return UIColor.black.withAlphaComponent(0.37)
+            case .black: return UIColor.black
+            case .white: return UIColor.white
+            }
         }
     }
 }

UIKitTests/UIScrollViewTests.swift

@@ -47,6 +47,42 @@ class UIScrollViewTests: XCTestCase {
         XCTAssertEqual(scrollView.contentInset, arbitraryContentInset)
     }
 
+    func testIndicatorsPositionInHierarchyWhenAddingSubviews() {
+        // subviews[0] is the back-most view, higher indexes are higher in layer hierarchy
+        // we want scroll indicators to always remain on top of 'normal' subviews
+        // and we want newer 'normal' subviews to be above older ones
+
+        // XXX: iOS does not allow explicit access to scroll indicators via subviews.
+        // We should remove that, too - but for now it's the easy way to go and helpful in testing
+
+        let mockView = UIView()
+        scrollView.addSubview(mockView)
+
+        XCTAssertGreaterThan(
+            scrollView.subviews.index(of: scrollView.horizontalScrollIndicator)!,
+            scrollView.subviews.index(of: mockView)!
+        )
+
+        XCTAssertGreaterThan(
+            scrollView.subviews.index(of: scrollView.verticalScrollIndicator)!,
+            scrollView.subviews.index(of: mockView)!
+        )
+    }
+
+    func testIndicatorsPositionInHierarchyWhenInsertingSubviewsAtAHighPosition() {
+        // If we try to insert at a position occupied by or above indicators,
+        // the inserted view should 'slide down' and assume the highest position below indicators
+
+        let mockViews = [UIView(), UIView(), UIView()]
+        for view in mockViews { scrollView.addSubview(view) }
+
+        let insertedMockView = UIView()
+        scrollView.insertSubview(insertedMockView, at: mockViews.count + 2)
+
+        XCTAssertEqual(scrollView.subviews.index(of: insertedMockView), mockViews.count)
+    }
+
+
     func testScrollIndicatorsVisibility() {
         //TODO: update to match iOS behaviour:
         //1. frame is always there, alpha changes from 0 to 1
@@ -98,10 +134,6 @@ class UIScrollViewTests: XCTestCase {
 
     // TODO: test final position. [blocked by setting contentOffset programatically - test behaviour in iOS]
 
-    func testIfScrollIndicatorsAreAccurate() {
-        // TODO: how?
-    }
-
     func testIfScrollViewBouncesBackAferPullIfNeeded() {
         // TODO: test setting inset and bouncing after it's implented in ScrollView
     }
@@ -158,11 +190,11 @@ class UIScrollViewTests: XCTestCase {
         wait(for: [beginDraggingExpectation, didScrollExpectation, didEndDraggingExpectation], timeout: 1.0)
     }
 
-    func mockTouch(toPoint: CGPoint, inScrollView scrollView: UIScrollView) {
+    func mockTouch(toPoint point: CGPoint, inScrollView scrollView: UIScrollView) {
         let mockTouch = UITouch(touchId: 0, at: CGPoint(x: 0, y: 0), timestamp: 0)
 
         scrollView.panGestureRecognizer.touchesBegan([mockTouch], with: UIEvent())
-        mockTouch.updateAbsoluteLocation(CGPoint(x: 100, y: 100))
+        mockTouch.updateAbsoluteLocation(point)
         scrollView.panGestureRecognizer.touchesMoved([mockTouch], with: UIEvent())
         scrollView.panGestureRecognizer.touchesEnded([mockTouch], with: UIEvent())
     }