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

add support for shareImage #457

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

add support for shareImage #457

wants to merge 9 commits into from

Conversation

typemytype
Copy link
Owner

No description provided.

"""
path = tempfile.mkstemp(suffix=f".{format}")[1]

self.saveImage(path, **kwargs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds like a job for saveImage("NSImage"). The tempfile just keeps hanging around.

Also: shouldn't this be a proper menu item rather than a library function?

Copy link
Owner Author

Choose a reason for hiding this comment

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

the cool thing to have this as library function is that it can be run from code outside the app.

@typemytype
Copy link
Owner Author

finally all test are green!

# share it over airdrop
shareImage('pdf', service="airdrop")
"""
path = tempfile.mkstemp(suffix=f".{format}")[1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Like I commented earlier, can we do this without a temp file? It's not being cleaned up, and I don't see how it could be. It's also not necessary as far as I can see: the sharing API works with all kinds of object, such as NSImage. Perhaps in-memory PDF data can be shared, too.

Copy link
Owner Author

Choose a reason for hiding this comment

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

A pdfDocument has no paste board support which is a requirement to send objects over.

The NSSharingService class is used to provide a consistent user experience when sharing items—NSURL objects, NSString objects, NSImage objects, video (through file URLs), or any object that implements the NSPasteboardWriting protocol—in macOS.

--> https://developer.apple.com/documentation/appkit/nssharingservice?language=objc

gif or mp4 has also no paste board support

its indeed not cleaned up, as it needs to be available during the sharing process

I could add a NSSharingServiceDelegate where a callback is received when the sharing is done

https://developer.apple.com/documentation/appkit/nssharingservicedelegate?language=objc

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then it indeed needs to be cleaned up with a delegate.

Copy link
Owner Author

Choose a reason for hiding this comment

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

this is silly: the delegate removes the assets to fast and fe mail can not find it anymore...

see https://gist.github.com/typemytype/ea02152e754a5b0225df2df23976afd5

"""
path = tempfile.mkstemp(suffix=f".{format}")[1]

context = getContextForFileExt("pdf")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Btw. why does this go via the pdf context and not via the regular saveImage()? Would this work for anything but pdf?

Copy link
Owner Author

Choose a reason for hiding this comment

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

cause it needs to be format this is wrong of me :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Question still stands: why not plain saveImage()?

Copy link
Owner Author

Choose a reason for hiding this comment

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

must be self.saveImage(..) indeed

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