Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In 1504: Readable Width for Saves and Search #763

Merged
merged 11 commits into from
May 31, 2023
Merged

In 1504: Readable Width for Saves and Search #763

merged 11 commits into from
May 31, 2023

Conversation

CMasterson
Copy link
Contributor

@CMasterson CMasterson commented May 30, 2023

Summary

Adds readable width to Saves and mimics it for Search.

References

https://getpocket.atlassian.net/browse/IN-1504

Implementation Details

Saves is built on UIKit and so we are able to add section.contentInsetsReference = .readableContent to the content cells layout config.

Search is built using SwiftUI, which does not appear to have a readableContent setting.
Therefore, if the interface width size class is .regular we restrict the list width to a preset amount (currently 500 points)

Test Steps

Launch and use Saves and Search as normal.
On iPad you may then enable split screen and while changing the window size you will notice the views adapt to their width.
iPhones do not have a landscape layout for Search and Saves, only Reader which already uses Readable Width.

PR Checklist:

  • Self Review (review, clean up, documentation, run tests)
  • Basic Self QA

Screenshots

Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-31 at 09 46 23
Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-31 at 09 46 17
Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-31 at 09 46 12
Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-31 at 09 46 04
Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-31 at 09 45 59
Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-31 at 09 45 51

@CMasterson CMasterson requested a review from a team as a code owner May 30, 2023 22:57
@CMasterson CMasterson requested review from cyndichin and Gio2018 and removed request for a team May 30, 2023 22:57
@pocket-ci
Copy link
Contributor

pocket-ci commented May 30, 2023

Messages
📖 No SwiftLint violations! 🎉
📖 Project coverage: 76.18%
📖 Checking XCode Environment Variables
📖 Edited 12 files
📖 Created 0 files

PocketKit: Coverage: 76.38

File Coverage
ItemsListViewController.swift 80.46%
SavedItemsListViewModel.swift 84.82%
ItemSkeletonCell.swift 94.67%
SearchView.swift 82.94%
ItemsListItemCell.swift 97.46%
ActivityViewController.swift 0.0% ⚠️
ItemsListItem.swift 100.0%
PocketItemViewModel.swift 90.3%
ItemsListOfflineCell.swift 94.44%
ItemsListItemPresenter.swift 83.33%
SavedItem+ItemsListItem.swift 80.0%
ItemsListViewModel.swift 50.0% ⚠️

Generated by 🚫 Danger Swift against 9455171

@CMasterson CMasterson changed the title In 1504 In 1504: Readable Width for Saves and Search May 31, 2023
@@ -325,6 +325,7 @@ extension ReadableViewController {
// Zero out the default leading/trailing contentInsets, but preserve the default top/bottom values.
// This ensures each section will be inset horizontally exactly to the readable content width.
var contentInsets = section.contentInsets
print("== contentInset \(contentInsets)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Contributor

@cyndichin cyndichin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one comment on print statement, otherwise code looks good!

@CMasterson
Copy link
Contributor Author

Updated the SwiftUI ReadableWidth mimic to 700 points, seems to work well on iPad.

@CMasterson CMasterson enabled auto-merge (squash) May 31, 2023 15:26
@CMasterson CMasterson merged commit 6029b64 into develop May 31, 2023
@CMasterson CMasterson deleted the IN-1504 branch May 31, 2023 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants