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

Fix #341 - restore support for animated gifs #342

Merged
merged 5 commits into from
Jun 14, 2021

Conversation

mr-fixit
Copy link
Contributor

@mr-fixit mr-fixit commented Feb 9, 2021

Added 2nd Target

(avoiding the '[target 2] has source overlapping sources' problem with a symlink)

    targets: [
        .target(
            name: "NYTPhotoViewer",
            path: "NYTPhotoViewer"
        ),
        .target(
            name: "NYTPhotoViewerGIF",
            dependencies: ["PINRemoteImage"],
            path: "SourceSymLink",
            cSettings: [
              .define("ANIMATED_GIF_SUPPORT", to: "1")
            ]
        )
    ]

Tweaked imports in 3 files by changing

#import <PINRemoteImage/FOO.h>

to

#if SWIFT_PACKAGE
  #import "FOO.h"
#else
  #import <PINRemoteImage/FOO.h>
#endif

@mr-fixit
Copy link
Contributor Author

mr-fixit commented Feb 9, 2021

I claim this fixes issue #341

@mr-fixit mr-fixit requested a review from noremac February 9, 2021 14:30
@noremac noremac removed their request for review March 9, 2021 20:53
@cassianodialpad
Copy link

@ZevEisenberg If you could please review this PR, it would be much appreciated! 😊

@cassianodialpad
Copy link

Bump! This would be very much appreciated! 😄

Copy link
Contributor

@ZevEisenberg ZevEisenberg left a comment

Choose a reason for hiding this comment

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

Sorry I dragged my feet so long on this. I thought I'd have to do a bunch of testing or something, but if this is working for people who need it to work, I'm happy with it!

@mr-fixit mr-fixit merged commit a4c0b51 into develop Jun 14, 2021
@mr-fixit mr-fixit deleted the bug/restore-support-for-animated-gifs branch June 14, 2021 13:20
@mr-fixit
Copy link
Contributor Author

@cassianodialpad, please let me know if i need to do something more to 'ship' this and make it accessible to you.

@ZevEisenberg
Copy link
Contributor

Probably need a new CocoaPods release to make it official

@mr-fixit
Copy link
Contributor Author

even though this is "just" an SPM thing?

@ZevEisenberg
Copy link
Contributor

Oh, good point

@cassianodialpad
Copy link

@mr-fixit I'm getting these errors on trying to build with SPM from the merge commit 0bdc8d3 :

SourcePackages/checkouts/NYTPhotoViewer/SourceSymLink/NYTScalingImageView.m:97:24: Receiver 'PINAnimatedImageView' for class message is a forward declaration
SourcePackages/checkouts/NYTPhotoViewer/SourceSymLink/NYTScalingImageView.m:97:23: Receiver type 'PINAnimatedImageView' for instance message is a forward declaration
SourcePackages/checkouts/NYTPhotoViewer/SourceSymLink/NYTScalingImageView.m:126:20: Property 'transform' cannot be found in forward class object 'PINAnimatedImageView'
SourcePackages/checkouts/NYTPhotoViewer/SourceSymLink/NYTScalingImageView.m:127:20: Property 'image' cannot be found in forward class object 'PINAnimatedImageView'
SourcePackages/checkouts/NYTPhotoViewer/SourceSymLink/NYTScalingImageView.m:131:20: Property 'animatedImage' cannot be found in forward class object 'PINAnimatedImageView'
SourcePackages/checkouts/NYTPhotoViewer/SourceSymLink/NYTScalingImageView.m:134:20: Property 'frame' cannot be found in forward class object 'PINAnimatedImageView'
SourcePackages/checkouts/NYTPhotoViewer/SourceSymLink/NYTScalingImageView.m:152:24: Property 'animatedImage' cannot be found in forward class object 'PINAnimatedImageView'
SourcePackages/checkouts/NYTPhotoViewer/SourceSymLink/NYTScalingImageView.m:152:56: Property 'image' cannot be found in forward class object 'PINAnimatedImageView'
SourcePackages/checkouts/NYTPhotoViewer/SourceSymLink/NYTScalingImageView.m:153:43: Property 'animatedImage' cannot be found in forward class object 'PINAnimatedImageView'
SourcePackages/checkouts/NYTPhotoViewer/SourceSymLink/NYTScalingImageView.m:153:74: Property 'animatedImage' cannot be found in forward class object 'PINAnimatedImageView'
SourcePackages/checkouts/NYTPhotoViewer/SourceSymLink/NYTScalingImageView.m:153:110: Property 'image' cannot be found in forward class object 'PINAnimatedImageView'

@mr-fixit
Copy link
Contributor Author

are you sure you have the right commit? the merge commit from this morning is a4c0b51

@cassianodialpad
Copy link

@mr-fixit Sorry, copy/paste issue... Yes, I tried commit a4c0b51 and got those errors on a clean build.

@mr-fixit
Copy link
Contributor Author

i didn't get the errors at first, but do now. looking into it. thanks for reporting!

@mr-fixit
Copy link
Contributor Author

i made #344

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