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

Getting ready to support AppKit #12

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

MojtabaHs
Copy link
Contributor

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

@Sherlouk
Copy link
Owner

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
Copy link
Owner

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!

@MojtabaHs
Copy link
Contributor Author

MojtabaHs commented Mar 23, 2024

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
Copy link
Contributor Author

MojtabaHs commented Apr 10, 2024

Any updates on this?

@Sherlouk
Copy link
Owner

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.

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.

2 participants