UI Improvements to the Main Collection View #405
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| // | ||
| // ChooseGameController+Prefetching.swift | ||
| // MAME4iOS | ||
| // | ||
| // Created by Yoshi Sugawara on 5/31/22. | ||
| // Copyright © 2022 MAME4iOS Team. All rights reserved. | ||
| // | ||
|
|
||
| extension ChooseGameController: UICollectionViewDataSourcePrefetching { | ||
| public func collectionView(_ collectionView: UICollectionView, prefetchItemsAt indexPaths: [IndexPath]) { | ||
| print("ChooseGameController: Start prefetch of images for indexPaths: \(indexPaths)") | ||
| for indexPath in indexPaths { | ||
| guard let game = getGameInfo(indexPath), | ||
|
Collaborator
There was a problem hiding this comment. I think you can just call the gameLoadImage (same thing when setting up cell) and ignore the result |
||
| let imageUrl = game.gameImageURLs.first else { | ||
| continue | ||
| } | ||
| guard ImageCache.sharedInstance().getImage(imageUrl) == nil else { | ||
ToddLa marked this conversation as resolved.
Show resolved
Hide resolved
Collaborator
There was a problem hiding this comment. We should add a NS_SWIFT_NAME so it is just ImageCache.shared |
||
| print("image already in cache, not fetching...") | ||
| continue | ||
| } | ||
| ImageCache.sharedInstance().getImage(imageUrl, localURL: game.gameLocalImageURL) { _ in | ||
| print("Prefetch: fetched image and put in cache") | ||
| } | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // | ||
| // Collection+Safe.swift | ||
| // MAME4iOS | ||
| // | ||
| // Created by Yoshi Sugawara on 5/31/22. | ||
| // Copyright © 2022 MAME4iOS Team. All rights reserved. | ||
| // | ||
|
|
||
| import Foundation | ||
|
|
||
| extension Collection { | ||
| /// Returns the element at the specified index if it is within bounds, otherwise nil. | ||
| subscript (safe index: Index) -> Element? { | ||
| return indices.contains(index) ? self[index] : nil | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| // | ||
| // RecentlyPlayedCell.swift | ||
| // MAME4iOS | ||
| // | ||
| // Created by Yoshi Sugawara on 5/29/22. | ||
| // Copyright © 2022 MAME4iOS Team. All rights reserved. | ||
| // | ||
|
|
||
| import UIKit | ||
|
|
||
| @objcMembers class RecentlyPlayedCell: UICollectionViewCell { | ||
|
Collaborator
There was a problem hiding this comment. I have a lot of experience with nested UICollectionView's and I am surprised how well they work, this looks good. I like how you did not duplicate all the cell setup, and just made this a container for "the same cells" from the parent. |
||
| static let identifier = "RecentlyPlayedCell" | ||
|
|
||
| var items = [GameInfo]() { | ||
| didSet { | ||
| collectionView.reloadData() | ||
| } | ||
| } | ||
|
|
||
| let collectionView: UICollectionView = { | ||
| let layout = UICollectionViewFlowLayout() | ||
| layout.scrollDirection = .horizontal | ||
| let collectionView = UICollectionView(frame: .zero, collectionViewLayout: layout) | ||
| return collectionView | ||
| }() | ||
|
|
||
| var itemSize = CGSize.zero { | ||
| didSet { | ||
| guard let layout = collectionView.collectionViewLayout as? UICollectionViewFlowLayout else { | ||
| return | ||
| } | ||
| layout.itemSize = itemSize | ||
| } | ||
| } | ||
| var setupCellClosure: ((GameInfoCell, IndexPath) -> Void)? | ||
| var selectItemClosure: ((IndexPath) -> Void)? | ||
| var contextMenuClosure: ((IndexPath) -> UIContextMenuConfiguration?)? | ||
| var heightForGameInfoClosure: ((GameInfo, UICollectionViewLayout) -> CGPoint)? | ||
|
|
||
| override init(frame: CGRect) { | ||
| super.init(frame: frame) | ||
| setupViews() | ||
| } | ||
|
|
||
| required init?(coder: NSCoder) { | ||
| fatalError("init(coder:) has not been implemented") | ||
| } | ||
|
|
||
| private func setupViews() { | ||
| backgroundColor = .clear | ||
| contentView.addSubview(collectionView) | ||
| collectionView.translatesAutoresizingMaskIntoConstraints = false | ||
| NSLayoutConstraint.activate([ | ||
|
Collaborator
There was a problem hiding this comment. Is This is really better than a A single line
|
||
| collectionView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor), | ||
| collectionView.trailingAnchor.constraint(equalTo: contentView.trailingAnchor), | ||
| collectionView.topAnchor.constraint(equalTo: contentView.topAnchor), | ||
| collectionView.bottomAnchor.constraint(equalTo: contentView.bottomAnchor) | ||
| ]) | ||
| collectionView.dataSource = self | ||
| collectionView.delegate = self | ||
| collectionView.register(GameInfoCell.self, forCellWithReuseIdentifier: Self.identifier) | ||
| } | ||
| } | ||
|
|
||
| extension RecentlyPlayedCell: UICollectionViewDataSource { | ||
| func numberOfSections(in collectionView: UICollectionView) -> Int { | ||
| return 1 | ||
| } | ||
|
|
||
| func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int { | ||
| return items.count | ||
| } | ||
|
|
||
| func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell { | ||
| guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: Self.identifier, for: indexPath) as? GameInfoCell else { | ||
| return UICollectionViewCell() | ||
| } | ||
| setupCellClosure?(cell, indexPath) | ||
| return cell | ||
| } | ||
| } | ||
|
|
||
| extension RecentlyPlayedCell: UICollectionViewDelegate { | ||
| func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) { | ||
| selectItemClosure?(indexPath) | ||
| } | ||
|
|
||
| func collectionView(_ collectionView: UICollectionView, contextMenuConfigurationForItemAt indexPath: IndexPath, point: CGPoint) -> UIContextMenuConfiguration? { | ||
| return contextMenuClosure?(indexPath) | ||
| } | ||
| } | ||
|
|
||
| extension RecentlyPlayedCell: UICollectionViewDelegateFlowLayout { | ||
| func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, sizeForItemAt indexPath: IndexPath) -> CGSize { | ||
| guard let layout = collectionViewLayout as? UICollectionViewFlowLayout, | ||
| let heightForGameInfoClosure = heightForGameInfoClosure, | ||
| let game = items[safe: indexPath.row] else { | ||
| return .zero | ||
| } | ||
| let heightInfo = heightForGameInfoClosure(game, layout) | ||
| let height = heightInfo.x + heightInfo.y | ||
| return CGSize(width: layout.itemSize.width, height: height) | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I am worried that iOS is gonna go crazy pre-loading and we will fire off a lot of net requests, we can't cancel