Add album list and basic album detail view #56
Conversation
danbee
force-pushed
the
add-ios-album-list
branch
from
60b344a to
23e5860
Compare
sharplet
left a comment
sharplet
left a comment
There was a problem hiding this comment.
Nice! This is very cool 👍
| guard let imageData = imageResult.image.pngData() | ||
| else { return } | ||
|
|
||
| let _ = RawImageDataProvider( |
There was a problem hiding this comment.
What does instantiating and then discarding the RawImageDataProvider do?
There was a problem hiding this comment.
Absolutely nothing. This was left over from some of the Mac code I copypasta'd
| } | ||
|
|
||
| override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) { | ||
| setAppearance() |
There was a problem hiding this comment.
FYI, to see if the trait collection change is related to dark mode or something else, you can use hasDifferentColorAppearance(comparedTo: previousTraitCollection).
| NotificationCenter.default.addObserver(self, selector: #selector(didConnect), name: .didConnect, object: nil) | ||
|
|
||
| title = "Albums" | ||
| navigationController?.navigationBar.prefersLargeTitles = true |
There was a problem hiding this comment.
FWIW if you're using a storyboard, it should be possible to set both of these properties directly in the storyboard file.
| albumMetadata.text = "\(album.artist) · \(album.date)" | ||
|
|
||
| var layoutSize = UIView.layoutFittingExpandedSize | ||
| layoutSize.width = UIScreen.main.bounds.width |
There was a problem hiding this comment.
FWIW, a good place to work out the layout width is in viewWillLayoutSubviews() or viewDidLayoutSubviews(). Moving this code there and using view.bounds.width instead of UIScreen.main.bounds.width will make this more robust to things like orientation changes or size changes on iPad.
There was a problem hiding this comment.
I moved it here because it was updating the layout often and I was noticing some jankyness with the calculation (the sizes of things would jump about). Now I've figured this it out I can probably move it back.
There was a problem hiding this comment.
Something I also usually do here is to cache the previous width, so the layout only needs to be recalculated if the width has changed.
|
|
||
| extension CGColor { | ||
| static let albumBorderColorLight = CGColor.init(srgbRed: 0, green: 0, blue: 0, alpha: 0.15) | ||
| static let albumBorderColorDark = CGColor.init(srgbRed: 255, green: 255, blue: 255, alpha: 0.15) |
There was a problem hiding this comment.
What do you think about defining these colours in an asset catalog so you can reuse them across platforms without dropping down to Core Graphics?
There was a problem hiding this comment.
I wasn't even aware that I could do that!
| // | ||
| // override func collectionView(_ collectionView: UICollectionView, indexPathForIndexTitle title: String, at index: Int) -> IndexPath { | ||
| // return IndexPath(index: 0) | ||
| // } |
There was a problem hiding this comment.
Can this commented code be removed?
There was a problem hiding this comment.
It can be for now. I was originally leaving it in as a reminder of the methods I need to implement to get the index thing working.
danbee
force-pushed
the
add-ios-album-list
branch
from
eeadba8 to
39f1c4f
Compare
There doesn't seem to be any overlap in the available methods for specifying a color on iOS or macOS.
Why wasn't I doing this before?
danbee
force-pushed
the
add-ios-album-list
branch
from
39f1c4f to
88ec059
Compare
Uh oh!
There was an error while loading. Please reload this page.