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

Implement native sharing #1158

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nightscape
Copy link
Contributor

@nightscape nightscape commented Nov 17, 2024

Closes #1090

  • Call handleShareTarget method when native sharing button is used
  • Return HTML to the client
  • Store shared data in a target page
  • Redirect user to target page
  • Make target page for shared data configurable
  • Allow using template variables in page name
  • Make it work in offline mode

@nightscape nightscape force-pushed the share_target branch 4 times, most recently from a739d91 to a87c6d4 Compare November 18, 2024 00:46
@nightscape nightscape marked this pull request as ready for review November 18, 2024 00:46
@nightscape
Copy link
Contributor Author

@zefhemel from my side this would be ok as a first shot. As a second step, it might be nice to allow adding tags to the added entries, but that could be done in a follow-up PR as I've mostly spent the time I'm able to invest in this feature.

@nightscape
Copy link
Contributor Author

Oh, the way it is currently implemented does not work in offline mode, which makes it 80% less useful...

@nightscape nightscape marked this pull request as draft November 18, 2024 06:54
Comment on lines +15 to +19
handleShareTarget:
path: share.ts:handleShareTarget
env: client
events:
- http:request:/share_target
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zefhemel I guess just specifying env: client here is not enough to trigger handleShareTarget through an incoming http request, right?
Is there already logic to delegate http:request events to plugs on the client side?
It looks like this would need to be done in service_worker.ts which currently only handles read requests.
Do you think it's a good idea to enhance service_worker.ts with dispatchEvent logic similar to http_server.ts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zefhemel ping 😉

Copy link
Collaborator

Choose a reason for hiding this comment

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

RIght. Right now handling HTTP requests through a service worker (so fully locally and therefore offline) is not possible. It's technically possible to do, but it's not there at this moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would service_worker.ts be the right place? Do you see any issues with using a similar approach as on the server-side, i.e. checking the plugs if one of them wants to handle the URL?

@nightscape nightscape marked this pull request as ready for review November 26, 2024 00:10
Copy link
Collaborator

@zefhemel zefhemel left a comment

Choose a reason for hiding this comment

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

Sorry for the huge delay of this.

I like this feature, but I'd implement it slightly differently, if this would work (I don't have an environment to test this on, because I think this is an Android feature only?) I'd implement the endpoint handling "hardcoded" in the service worker. That is: in the fetch event handler, decide on some URL (like /.share or something), and put the logic in the service worker directly rather than in the share plug. I feel this is functionality that deserves a place directly in the SB core, not so much in a plug (it needs to be hardcoded in the manifest anyway), and implemented in the service worker it can also be done (I think) in a way that works both in online and offline mode.

@zefhemel
Copy link
Collaborator

A head's up: working with service worker is a bit of a pain in terms of reload behavior. In Chrome, you can have the service worker update on every reload:

CleanShot 2024-12-14 at 09 52 48@2x

Still you probably need to reload your page a few times after every change to make sure you got the new service worker running before deciding things don't work ;)

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.

Feature request: Share target
2 participants