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.

Sign up for GitHub

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

Jump to bottom

Update carthage files, including our custom NYTPhotoViewer #3812

Closed
wants to merge 2 commits into from
Closed

Update carthage files, including our custom NYTPhotoViewer #3812

wants to merge 2 commits into from

Conversation

mcleinman
Copy link
Contributor

Fixes Phabricator ticket: https://phabricator.wikimedia.org/T249274

Notes

  • The biggest concern here is the NYTPhotoViewer update. We've brought our custom one years of updates, and maintained our special display of the navbar. (I had to update some of our code to work w/ updated NYTPhotoViewer method signatures.)
  • I checked for memory leaks, doesn't seem to have any.

Order of operations for this review/merge:

  1. This PR should be checked
  2. Merge in Update from upstream NYTPhotoViewer#2
  3. Update the Cartfile in this branch to point to the proper NYTPhotoViewer branch
  4. Check this PR again, merge it

Test Steps

  1. Ensure Picture of the Day works.
  2. Ensure images in article galleries work, including scrolling between them.
  3. Nothing should be different than how the app currently operates.

@mcleinman mcleinman requested review from a team and staykids and removed request for a team December 23, 2020 20:39
mcleinman
mcleinman commented Dec 23, 2020
View reviewed changes
Wikipedia/Code/WMFImageGalleryViewController.m
@@ -127,9 +131,7 @@ - (void)setOverlayViewTopBarHidden:(BOOL)hidden {
if (hidden) {
self.overlayView.rightBarButtonItem = nil;
self.overlayView.leftBarButtonItem = nil;
self.overlayView.topCoverBackgroundColor = [UIColor clearColor];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was for our custom iPhone X work; no longer needed.

@staykids
Copy link
Contributor

staykids commented Jan 5, 2021

On hold as we track:

@staykids staykids added the Hold label Jan 5, 2021
@staykids staykids removed their request for review January 5, 2021 23:05
@staykids
Copy link
Contributor

staykids commented Mar 2, 2021

Closing due to Carthage removal.

@staykids staykids closed this Mar 2, 2021
@tonisevener tonisevener deleted the update-nytphotoviewer branch May 24, 2021 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mcleinman @staykids