Fix: Collection view focus #102
Conversation
3lvis
changed the title
|
@marijnschilling what about this one? |
|
I feel like I cant really review it, really dont know what is going on in the code :( |
|
@marijnschilling Maybe I can help, anything in special that feels difficult to understand? |
|
Yeah maybe explain what changed code-wise. In the context of the classes that are changed. I also don't see from the gifs what has changed in functionality. |
|
@marijnschilling For the gif, check the last part when the viewer is dismissed, then you will see the difference. In one it goes back to the place that it should go and in the other one it jumps to another cell. |
| @@ -50,6 +50,20 @@ class PhotosController: UICollectionViewController { | |||
| } | |||
| } | |||
|
|
|||
| #if os(tvOS) | |||
| override var preferredFocusEnvironments: [UIFocusEnvironment] { | |||
There was a problem hiding this comment.
In this class the main changes are improving the control of the focus engine. preferredFocusEnvironments should return the views that you want to focus, in our case it will return the currently selected cell.
| self.present(self.viewerController!, animated: false, completion: nil) | ||
| } | ||
|
|
||
| #if os(tvOS) | ||
| override public func collectionView(_ collectionView: UICollectionView, canFocusItemAt indexPath: IndexPath) -> Bool { |
There was a problem hiding this comment.
This is where we tell the collection view if it should focus or not its items.
There was a problem hiding this comment.
If the viewer is dismissed it will work as usual, otherwise it ignores the focus changes, this disables the default behavior where the collection view will try to figure out which view to focus on, and instead we will tell the collection view which view to focus thanks to the preferredFocusEnvironments.
There was a problem hiding this comment.
I don't really understand what these focus changes are.. and UIFocusEnvironment I might have to read up on that before i understand what it does.
| extension PhotosController: ViewerControllerDelegate { | ||
| func viewerController(_ viewerController: ViewerController, didChangeFocusTo indexPath: IndexPath) {} | ||
|
|
||
| func viewerControllerDidDismiss(_ viewerController: ViewerController) { |
There was a problem hiding this comment.
When the controller is dismissed we tell the collection view to figure out what do focus on, this will trigger a call to preferredFocusEnvironments.
| @@ -94,12 +94,12 @@ public class ViewerController: UIViewController { | |||
| /** | |||
There was a problem hiding this comment.
Changes on ViewerController are mainly to support the functionality on PhotosController.
|
@marijnschilling let me know if I can clarify anything else. |
|
Gonna merge this to update the whole project to Swift 3.1. Will continue with code-review comments on a new PR. |
|
|
||
| #if os(tvOS) | ||
| override public func collectionView(_ collectionView: UICollectionView, canFocusItemAt indexPath: IndexPath) -> Bool { | ||
| let isViewerIsVisible = self.viewerController?.isPresented ?? false |
There was a problem hiding this comment.
I think this should be called isViewerVisible right?
| self.present(self.viewerController!, animated: false, completion: nil) | ||
| } | ||
|
|
||
| #if os(tvOS) | ||
| override public func collectionView(_ collectionView: UICollectionView, canFocusItemAt indexPath: IndexPath) -> Bool { |
There was a problem hiding this comment.
I don't really understand what these focus changes are.. and UIFocusEnvironment I might have to read up on that before i understand what it does.
|
|
||
| /** | ||
| A helper to prevent the paginated scroll view to be set up twice when is presented | ||
| */ | ||
| fileprivate var presented = false | ||
| fileprivate(set) public var isPresented = false |
There was a problem hiding this comment.
Oooh what does this do? fileprivate(set) public
There was a problem hiding this comment.
Means you can read it but not write on it.
|
The code looks good! |
Viewer currently wasn't handling focus quite well changing the focus of the selected cell when you swipe between viewable objects, this PR aims to fix that.
Before
After