Skip to content

Conversation

@jwrubel
Copy link
Contributor

@jwrubel jwrubel commented Dec 18, 2018

Hi! I needed these two services for a 'read later' function so I added them. I've checked the demo page and they look ok to me. I used PR #181 as a reference here - Let me know if I missed anything.

Cheers, and thanks for making this awesome library available!

@jwrubel
Copy link
Contributor Author

jwrubel commented Dec 19, 2018

this is the result of running the demos for me, with the two new icons at the end
image 2018-12-18 at 19 54 22


return 'http://www.instapaper.com/hello2' + objectToGetParams({
url,
text: title,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should the name of the parameter be title?

I did not find the official documentation, but https://feedbin.com/help/sharing-read-it-later-services/ and https://gist.github.com/revelt/a70923df337f7923e067 suggest that url and title would be the parameters to use here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found the documentation and title is correct, plus it allows a description property. I'll add these.


return 'https://getpocket.com/save' + objectToGetParams({
url,
text: title,
Copy link
Collaborator

Choose a reason for hiding this comment

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

The same question over here: should the query parameter be named title? The links mentioned in the previous comment provide examples for Pocket. There it seems that title is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! After reviewing the documentation for their API it looks like the title parameter is ignored if Pocket can get a title parameter from the shared page, so it looked like it was working for me when I tested. I'll make this change, and also I notice that Pocket supports a tags property and also a tweet_id so I'll update to add those.

@jwrubel
Copy link
Contributor Author

jwrubel commented Dec 23, 2018

okay @aautio I've updated the code to reflect title as the parameter name for both services and added a description property for Instapaper. After some research, it appears that the bookmarklet link we're using for pocket here doesn't accept tags or tweet_id if they are passed. Only the Pocket API allows this.

@cedricdelpoux
Copy link

It will be awesome when this PR will be merged 🎉

@aautio aautio changed the title add Pocket and Instagram share buttons and icons add Pocket and Instapaper share buttons and icons Feb 16, 2019
@aautio aautio changed the base branch from master to v3 February 16, 2019 12:25
…jwrubel-master

# Conflicts:
#	README.md
#	docs/index.html
#	docs/main.e2b9c3407372f202448e.bundle.js
@aautio aautio merged commit 22d35ac into nygardk:v3 Feb 16, 2019
@aautio aautio mentioned this pull request Feb 16, 2019
@aautio
Copy link
Collaborator

aautio commented Feb 16, 2019

Thank you @jwrubel 👍

You can follow #210 to see when the Pocket and Instapaper -additions are published to npm.

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