Feature/scroll to view controller #19
Changes from 14 commits
7c428d5
acc5db1
29fadc1
60eba94
321d7a3
526159d
5ca2917
80eb8d6
a410445
c2c0399
e63e952
6fc3328
f2e455d
4fd8a01
280625d
c31c61d
7c547c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// | ||
// Focusable.swift | ||
// Matrioska | ||
// | ||
// Created by Andreas Thenn on 14/03/2017. | ||
// Copyright © 2017 runtastic. All rights reserved. | ||
// | ||
|
||
import Foundation | ||
|
||
/** This protocol should be implemented by every UIViewController that has some kind of focus, | ||
highlight or select method. We want these to be adressable in a more generic way and | ||
we don't need to care about the actual underlying method that's being called. | ||
*/ | ||
protocol Focusable { | ||
|
||
/** Try to focus on the given UIViewController, otherwise it fails silently */ | ||
@discardableResult | ||
func focus(on viewController: UIViewController) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I second @joanromano's position: "I would expect Matrioska to support navigation to some Component or child Component in a generic way". Matrioska is basically a " But before that, I think we need to introduce an identity concept for The question is also whether the focus request should be dispatched to the root of the
or the
To me it looks like the focusing will require a top-down approach anyway (stuff higher up the hierarchy potentially needs to be scrolled into position before focusing a nested child), so the first alternative seems the more natural thing to do. |
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,8 +9,16 @@ | |
import UIKit | ||
import SnapKit | ||
|
||
/// An enum that describes the relative position of an element. The enum should work horizontally as well as vertically. | ||
/// Orientation depending meaning: begin - top/left, end - bottom/right | ||
public enum RelativePosition { | ||
case begin | ||
case center | ||
case end | ||
} | ||
|
||
/// A ViewController that contains a stackView | ||
final class StackViewController: UIViewController { | ||
final class StackViewController: UIViewController, Focusable { | ||
|
||
private let preserveParentWidth: Bool | ||
private let scrollView = IntrinsicSizeAwareScrollView() | ||
|
@@ -24,7 +32,7 @@ final class StackViewController: UIViewController { | |
self.preserveParentWidth = configuration.preserveParentWidth | ||
|
||
super.init(nibName: nil, bundle: nil) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. try to avoid these next time ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we removed trailing space from swiftlint and @AndreasThenn has a different Xcode setting |
||
title = configuration.title | ||
stackView.distribution = .equalSpacing | ||
stackView.axis = configuration.axis | ||
|
@@ -33,7 +41,7 @@ final class StackViewController: UIViewController { | |
if !preserveParentWidth { | ||
stackView.alignment = .leading | ||
} | ||
|
||
scrollView.backgroundColor = configuration.backgroundColor | ||
} | ||
|
||
|
@@ -69,4 +77,65 @@ final class StackViewController: UIViewController { | |
stackView.addArrangedSubview(childViewController.view) | ||
childViewController.didMove(toParentViewController: self) | ||
} | ||
|
||
public func focus(on viewController: UIViewController) { | ||
scroll(to: viewController, at: .center, animated: true) | ||
} | ||
|
||
/// A method to access the scrollView from outside and scroll to a given target ViewController. | ||
/// If no relativePosition parameter is provided, it will try to center the target. | ||
/// | ||
/// - Parameters: | ||
/// - targetViewController: The ViewController which should scroll into focus | ||
/// - position: An optional parameter to specify the alignement of the target. Depending of | ||
/// the stackViews' orientation position values have different meaning: | ||
/// begin - top/left, end - bottom/right | ||
private func scroll(to targetViewController: UIViewController, | ||
at position: RelativePosition = .center, | ||
animated: Bool = false) { | ||
|
||
guard childViewControllers.contains(targetViewController) else { | ||
return | ||
} | ||
|
||
var targetOffset = CGPoint.zero | ||
let targetFrame = targetViewController.view.frame | ||
|
||
if stackView.axis == .vertical { | ||
// keep current horizontal offset | ||
targetOffset.x = scrollView.contentOffset.x | ||
let topOffset = targetFrame.origin.y - scrollView.contentInset.top | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move the calculation in the respected case |
||
let bottomOffset = targetFrame.maxY - scrollView.bounds.height | ||
let maxOffset = scrollView.contentSize.height - scrollView.bounds.height | ||
|
||
switch position { | ||
case .begin: | ||
targetOffset.y = topOffset | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about |
||
case .center: | ||
targetOffset.y = topOffset - (topOffset - bottomOffset) / 2 | ||
case .end: | ||
targetOffset.y = bottomOffset | ||
} | ||
|
||
targetOffset.y = min(maxOffset, max(scrollView.contentInset.top, targetOffset.y)) | ||
} else { | ||
// keep current vertical offset | ||
targetOffset.y = scrollView.contentOffset.y | ||
let leftOffset = targetFrame.origin.x - scrollView.contentInset.left | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as vertical, move it in the cases There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ChristianOrgler Why? I then I need to duplicate code, that I need in multiple cases. |
||
let rightOffset = targetFrame.maxX - scrollView.bounds.width | ||
let maxOffset = scrollView.contentSize.width - scrollView.bounds.width | ||
switch position { | ||
case .begin: | ||
targetOffset.x = leftOffset | ||
case .center: | ||
targetOffset.x = leftOffset - (leftOffset - rightOffset) / 2 | ||
case .end: | ||
targetOffset.x = rightOffset | ||
} | ||
|
||
targetOffset.x = min(maxOffset, max(scrollView.contentInset.left, targetOffset.x)) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Had a general look here and also seems to me that some |
||
|
||
scrollView.setContentOffset(targetOffset, animated: animated) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@discardableResult
on a function with return typeVoid
is kind of pointless, wouldn't you agree?