swift-snapshot-testing-stitch icon indicating copy to clipboard operation
swift-snapshot-testing-stitch copied to clipboard

Getting ready to support AppKit

Open MojtabaHs opened this issue 1 year ago • 5 comments
trafficstars

The first step is to use AppKit counterparts.

  • The iOS stays the same
  • Dependencies (SnapshotTestingHEIC) may need to be updated before going further
  • Will add the macOS platform after everything checked

MojtabaHs avatar Mar 21 '24 14:03 MojtabaHs

This is awesome. I know SnapshotTestingHEIC and SnapshotTesting itself both support macOS, so would be great to support macOS here too.

Will try and loop around this weekend to test these changes out before merging 👍

Sherlouk avatar Mar 21 '24 14:03 Sherlouk

Just taken a quick look, and does look like there's a little more here to figure out for macOS.

  • There's a missing capability in SnapshotTestingHEIC which means you can't currently route compressionQuality through to an NSView (but can to an NSImage). This isn't a blocker, but something I'll fix upstream before releasing this. For now we can just ignore this parameter (but will require a compiler flag around it's usage in Snapshotting).
  • We're currently using UIGraphicsImageRenderer as the sole mechanism for stitching images. We'd need to figure out a macOS alternative before continuing with this pull request.
  • Both upstream dependencies are slightly outdated, but this shouldn't block this PR from what I'm seeing - and I can tackle these separately before publishing a release. Don't feel like you need to do these!

Sherlouk avatar Mar 23 '24 18:03 Sherlouk

Just taken a quick look, and does look like there's a little more here to figure out for macOS.

  • There's a missing capability in SnapshotTestingHEIC which means you can't currently route compressionQuality through to an NSView (but can to an NSImage). This isn't a blocker, but something I'll fix upstream before releasing this. For now we can just ignore this parameter (but will require a compiler flag around it's usage in Snapshotting).
  • We're currently using UIGraphicsImageRenderer as the sole mechanism for stitching images. We'd need to figure out a macOS alternative before continuing with this pull request.
  • Both upstream dependencies are slightly outdated, but this shouldn't block this PR from what I'm seeing - and I can tackle these separately before publishing a release. Don't feel like you need to do these!

Thanks for reviewing and accepting this. I see this as a first step and there are definitely more (including all the points you precisely mentioned). I would be glad to contribute to the next steps as well and make this cool package truly compatible and ready for the AppKit 🙌🏻

MojtabaHs avatar Mar 23 '24 19:03 MojtabaHs

Any updates on this?

MojtabaHs avatar Apr 10 '24 07:04 MojtabaHs

As above, I would like for the pull request to complete the implementation before merging.

Of course, if you'd like to create separate pull requests merging into an epic branch then we can do this also - but I'd like to keep main complete until the changes can be reviewed as a whole.

Sherlouk avatar Apr 10 '24 13:04 Sherlouk