fix(reader): makes reader use readable content width #416
Conversation
| @@ -93,6 +93,12 @@ class ReadableViewController: UIViewController { | |||
| readableViewModel.fetchDetailsIfNeeded() | |||
| } | |||
|
|
|||
| override func viewWillTransition(to size: CGSize, with coordinator: UIViewControllerTransitionCoordinator) { | |||
There was a problem hiding this comment.
Very subtly you can see that images may use more space then they should after a rotation, this attempts to correct that by making the collection view re-layout after a size change.
| @@ -284,20 +287,26 @@ extension ReadableViewController { | |||
| ) | |||
|
|
|||
| let section = NSCollectionLayoutSection(group: group) | |||
| section.contentInsets = Constants.metaSectionContentInsets | |||
| // Zero out the default leading/trailing contentInsets, but preserve the default top/bottom values. | |||
There was a problem hiding this comment.
Used https://developer.apple.com/forums/thread/679863 as my guide.
| var contentInsets = section.contentInsets | ||
| contentInsets.leading = 0 | ||
| contentInsets.trailing = 0 | ||
| contentInsets.top = Constants.contentSectionContentInsets.top | ||
| contentInsets.bottom = Constants.metaSectionContentInsets.bottom | ||
| section.contentInsets = contentInsets | ||
| section.contentInsetsReference = .readableContent |
There was a problem hiding this comment.
nit: similar as logic above, should we put this in a private func, so we only have to change in one place
There was a problem hiding this comment.
im gonna leave that change out for now given it uses different constants,
|
|
||
|
|
There was a problem hiding this comment.
just another note, should we enforce SwiftLint rules instead of having them as warnings? I notice at times we merge them in
There was a problem hiding this comment.
@cyndichin yes we should.
PocketKit: Coverage: 75.2
Generated by 🚫 Danger Swift against 745b537 |
Summary
Makes the reader user-readable content width so that the reader looks better on devices like the iPad.
References
Implementation Details
Was not sure what tests I could use here.
Test Steps
PR Checklist:
Screenshots
Before:
After:
