Merge pull request #63 from readium/fixes/scrollmode

Fix EPUB chapter positionning when scroll mode is enabled

Author: aferditamuriqi

r2-navigator-swift/EPUB/DocumentWebView.swift

@@ -16,8 +16,10 @@ import R2Shared
 protocol DocumentWebViewDelegate: class {
     func willAnimatePageChange()
     func didEndPageAnimation()
-    func displayRightDocument(animated: Bool, completion: @escaping () -> Void)
-    func displayLeftDocument(animated: Bool, completion: @escaping () -> Void)
+    @discardableResult
+    func displayRightDocument(animated: Bool, completion: @escaping () -> Void) -> Bool
+    @discardableResult
+    func displayLeftDocument(animated: Bool, completion: @escaping () -> Void) -> Bool
     func webView(_ webView: DocumentWebView, didTapAt point: CGPoint)
     func handleTapOnLink(with url: URL)
     func handleTapOnInternalLink(with href: String)
@@ -218,51 +220,66 @@ class DocumentWebView: UIView, Loggable {
     /// - Parameter body: Unused.
     private func documentDidLoad(body: Any) {
         documentLoaded = true
-        
+
+        applyUserSettingsStyle()
+
         // FIXME: We need to give the CSS and webview time to layout correctly. 0.2 seconds seems like a good value for it to work on an iPhone 5s. Look into solving this better
         DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) {
-            self.activityIndicatorView?.stopAnimating()
-            UIView.animate(withDuration: self.animatedLoad ? 0.3 : 0, animations: {
-                self.scrollView.alpha = 1
-            })
+            self.scrollToInitialPosition {
+                self.activityIndicatorView?.stopAnimating()
+                UIView.animate(withDuration: self.animatedLoad ? 0.3 : 0, animations: {
+                    self.scrollView.alpha = 1
+                })
+            }
         }
-
-        applyUserSettingsStyle()
-        scrollToInitialPosition()
     }
-    
+
     // Scroll at position 0-1 (0%-100%)
-    func scrollAt(position: Double) {
-        guard position >= 0 && position <= 1 else { return }
-        
-        let dir = readingProgression.rawValue
-        evaluateScriptInResource("readium.scrollToPosition(\'\(position)\', \'\(dir)\')")
+    func scrollAt(position: Double, completion: @escaping () -> Void = {}) {
+        guard position >= 0 && position <= 1 else {
+            log(.warning, "Scrolling to invalid position \(position)")
+            completion()
+            return
+        }
+
+        // Note: The JS layer does not take into account the scroll view's content inset. So it can't be used to reliably scroll to the top or the bottom of the page in scroll mode.
+        if isScrollEnabled && [0, 1].contains(position) {
+            var contentOffset = scrollView.contentOffset
+            contentOffset.y = (position == 0)
+                ? -scrollView.contentInset.top
+                : (scrollView.contentSize.height - scrollView.bounds.height + scrollView.contentInset.bottom)
+            scrollView.contentOffset = contentOffset
+            completion()
+        } else {
+            let dir = readingProgression.rawValue
+            evaluateScriptInResource("readium.scrollToPosition(\'\(position)\', \'\(dir)\')") { _, _ in completion () }
+        }
     }
 
     // Scroll at the tag with id `tagId`.
-    func scrollAt(tagId: String) {
-        evaluateScriptInResource("readium.scrollToId(\'\(tagId)\');")
+    func scrollAt(tagId: String, completion: @escaping () -> Void = {}) {
+        evaluateScriptInResource("readium.scrollToId(\'\(tagId)\');") { _, _ in completion() }
     }
 
     // Scroll to .beggining or .end.
-    func scrollAt(location: BinaryLocation) {
+    func scrollAt(location: BinaryLocation, completion: @escaping () -> Void = {}) {
         switch location {
         case .left:
-            scrollAt(position: 0)
+            scrollAt(position: 0, completion: completion)
         case .right:
-            scrollAt(position: 1)
+            scrollAt(position: 1, completion: completion)
         }
     }
 
     /// Moves the webView to the initial location.
-    func scrollToInitialPosition() {
+    func scrollToInitialPosition(completion: @escaping () -> Void = {}) {
         /// If the savedProgression property has been set by the navigator.
         if let initialPosition = progression, initialPosition > 0.0 {
-            scrollAt(position: initialPosition)
+            scrollAt(position: initialPosition, completion: completion)
         } else if let initialId = initialId {
-            scrollAt(tagId: initialId)
+            scrollAt(tagId: initialId, completion: completion)
         } else {
-            scrollAt(location: initialLocation)
+            scrollAt(location: initialLocation, completion: completion)
         }
     }
 
@@ -272,7 +289,18 @@ class DocumentWebView: UIView, Loggable {
     }
 
     func scrollTo(_ direction: ScrollDirection, animated: Bool = false, completion: @escaping () -> Void = {}) -> Bool {
-        let viewDelegate = self.viewDelegate
+        if isScrollEnabled {
+            guard let viewDelegate = viewDelegate else {
+                return false
+            }
+            switch direction {
+            case .left:
+                return viewDelegate.displayLeftDocument(animated: animated, completion: completion)
+            case .right:
+                return viewDelegate.displayRightDocument(animated: animated, completion: completion)
+            }
+        }
+        
         if animated {
             switch direction {
             case .left:
@@ -295,17 +323,17 @@ class DocumentWebView: UIView, Loggable {
         let dir = readingProgression.rawValue
         switch direction {
         case .left:
-            evaluateScriptInResource("readium.scrollLeft(\"\(dir)\");") { result, error in
+            evaluateScriptInResource("readium.scrollLeft(\"\(dir)\");") { [weak self] result, error in
                 if error == nil, let success = result as? Bool, !success {
-                    viewDelegate?.displayLeftDocument(animated: animated, completion: completion)
+                    self?.viewDelegate?.displayLeftDocument(animated: animated, completion: completion)
                 } else {
                     completion()
                 }
             }
         case .right:
-            evaluateScriptInResource("readium.scrollRight(\"\(dir)\");") { result, error in
+            evaluateScriptInResource("readium.scrollRight(\"\(dir)\");") { [weak self] result, error in
                 if error == nil, let success = result as? Bool, !success {
-                    viewDelegate?.displayRightDocument(animated: animated, completion: completion)
+                    self?.viewDelegate?.displayRightDocument(animated: animated, completion: completion)
                 } else {
                     completion()
                 }

r2-navigator-swift/EPUB/EPUBNavigatorViewController.swift

@@ -334,15 +334,17 @@ extension EPUBNavigatorViewController: DocumentWebViewDelegate {
     }
 
     /// Display next document (readingOrder item).
-    func displayRightDocument(animated: Bool, completion: @escaping () -> Void) {
+    @discardableResult
+    func displayRightDocument(animated: Bool, completion: @escaping () -> Void) -> Bool {
         let delta = triptychView.readingProgression == .rtl ? -1 : 1
-        goToIndex(triptychView.index + delta, animated: animated, completion: completion)
+        return goToIndex(triptychView.index + delta, animated: animated, completion: completion)
     }
 
     /// Display previous document (readingOrder item).
-    func displayLeftDocument(animated: Bool, completion: @escaping () -> Void) {
+    @discardableResult
+    func displayLeftDocument(animated: Bool, completion: @escaping () -> Void) -> Bool {
         let delta = triptychView.readingProgression == .rtl ? -1 : 1
-        goToIndex(triptychView.index - delta, animated: animated, completion: completion)
+        return goToIndex(triptychView.index - delta, animated: animated, completion: completion)
     }
 
 }

r2-navigator-swift/EPUB/ReflowableDocumentWebView.swift

@@ -24,7 +24,7 @@ final class ReflowableDocumentWebView: DocumentWebView {
     override func setupWebView() {
         super.setupWebView()
         scrollView.bounces = false
-        scrollView.isPagingEnabled = true
+        scrollView.isPagingEnabled = !isScrollEnabled
 
         webView.translatesAutoresizingMaskIntoConstraints = false
         topConstraint = webView.topAnchor.constraint(equalTo: topAnchor)
@@ -63,19 +63,20 @@ final class ReflowableDocumentWebView: DocumentWebView {
     override func applyUserSettingsStyle() {
         super.applyUserSettingsStyle()
 
-        if let userSettings = userSettings {
-            for cssProperty in userSettings.userProperties.properties {
+        if let properties = userSettings?.userProperties.properties {
+            let propertiesScript = properties.reduce("") { script, property in
                 let value: String = {
                     // Scroll mode depends both on the user settings, and on the fact that VoiceOver is activated or not, so we need to generate the value dynamically.
                     // FIXME: This would be handled in a better way by decoupling the user settings from the actual ReadiumCSS properties sent to the WebView, which should be private details of the EPUBNavigator implementation and not shared with the host app.
-                    if let switchable = cssProperty as? Switchable, cssProperty.name == ReadiumCSSName.scroll.rawValue {
+                    if let switchable = property as? Switchable, property.name == ReadiumCSSName.scroll.rawValue {
                         return switchable.values[isScrollEnabled]!
                     } else {
-                        return cssProperty.toString()
+                        return property.toString()
                     }
                 }()
-                evaluateScriptInResource("readium.setProperty(\"\(cssProperty.name)\", \"\(value)\");")
+                return script + "readium.setProperty(\"\(property.name)\", \"\(value)\");\n"
             }
+            evaluateScriptInResource(propertiesScript)
         }
 
         // Disables paginated mode if scroll is on.
@@ -85,20 +86,21 @@ final class ReflowableDocumentWebView: DocumentWebView {
     }
 
     private func updateContentInset() {
-        var insets = contentInset[traitCollection.verticalSizeClass]
-            ?? contentInset[.regular]
-            ?? contentInset[.unspecified]
-            ?? (top: 0, bottom: 0)
-        
-        // Increases the insets by the notch area (eg. iPhone X) to make sure that the content is not overlapped by the screen notch.
-        insets.top += notchAreaInsets.top
-        insets.bottom += notchAreaInsets.bottom
-
         if (isScrollEnabled) {
             topConstraint.constant = 0
             bottomConstraint.constant = 0
-            scrollView.contentInset = UIEdgeInsets(top: insets.top, left: 0, bottom: insets.bottom, right: 0)
+            scrollView.contentInset = UIEdgeInsets(top: notchAreaInsets.top, left: 0, bottom: notchAreaInsets.bottom, right: 0)
+            
         } else {
+            var insets = contentInset[traitCollection.verticalSizeClass]
+                ?? contentInset[.regular]
+                ?? contentInset[.unspecified]
+                ?? (top: 0, bottom: 0)
+            
+            // Increases the insets by the notch area (eg. iPhone X) to make sure that the content is not overlapped by the screen notch.
+            insets.top += notchAreaInsets.top
+            insets.bottom += notchAreaInsets.bottom
+            
             topConstraint.constant = insets.top
             bottomConstraint.constant = -insets.bottom
             scrollView.contentInset = .zero

r2-navigator-swift/EPUB/TriptychView.swift

@@ -97,7 +97,14 @@ final class TriptychView: UIView {
     }
 
     /// Index of the document currently being displayed.
-    fileprivate(set) var index: Int
+    fileprivate(set) var index: Int {
+        willSet {
+            guard let cw = currentView as? DocumentWebView else {
+                return
+            }
+            cw.scrollAt(location: (index < newValue) ? trailing : leading)
+        }
+    }
 
     fileprivate let scrollView: UIScrollView
 
@@ -357,11 +364,9 @@ extension TriptychView {
         if index < nextIndex {
             currentRect.x += coefficient*currentFrameSize.width
             scrollView.scrollRectToVisible(CGRect(origin: currentRect, size: currentFrameSize), animated: false)
-            cw.scrollAt(location: trailing)
         } else {
             currentRect.x -= coefficient*currentFrameSize.width
             scrollView.scrollRectToVisible(CGRect(origin: currentRect, size: currentFrameSize), animated: false)
-            cw.scrollAt(location: leading)
         }
 
         let previousIndex = index

r2-navigator-swift/EPUB/UserSettings.swift

@@ -16,8 +16,9 @@ import R2Shared
 
 public class UserSettings {
 
+    // WARNING: String values must not contain any single or double quotes characters, otherwise it breaks the streamer's injection.
     private let appearanceValues = ["readium-default-on", "readium-sepia-on","readium-night-on"]
-    private let fontFamilyValues = ["Publisher's default", "Helvetica Neue", "Iowan Old Style", "Athelas", "Seravek", "OpenDyslexic"]
+    private let fontFamilyValues = ["Original", "Helvetica Neue", "Iowan Old Style", "Athelas", "Seravek", "OpenDyslexic"]
     private let textAlignmentValues = ["justify", "start"]
     private let columnCountValues = ["auto", "1", "2"]