Address #70, add template variable pocket-url #89

ashlyn · 2022-04-01T15:10:45Z

Relevant Changes

Adds a new template variable, {{pocket-url}} to open the Pocket item inside the Pocket UI. The URL is constructed using the item_id field and relies on the user being authenticated to Pocket in their browser. It also has the potential to break if Pocket updates the schema for item URLs.
Adds two links next to titles in the Pocket item list view, one to open the original, one to open in Pocket. The alt-click functionality to open the original URL when click on the title is preserved. See screenshot for an example.
I've updated the versions by one minor version (assuming semantic versioning) in the manifest.json and package.json as well as adding the new variable to the README.

With New Links

Example with Long Titles/Narrow Screen Width

Addresses #70.

Testing Steps

Basic Running Instructions

Clone this branch into the .obsidian/plugins directory in a vault. It may be easiest to use a test vault, especially if you also have the current version of this plugin installed.
Run the npm i command to install Node packages, then npm run dev to start a dev server with a build watch.
Enable this plugin in the Obsidian settings and make sure it is the only version installed in the vault: the version should read as 0.7.1
Configure the authentication either by using the Connect Your Pocket Account in the plugin settings or by copying an existing, valid .__pocket_access_info__ file into the root directory.
You may find this Obsidian Hot Reload plugin helpful for testing

Branch Specific Instructions

If this vault doesn't have a Pocket list already, ensure your Pocket account has items to sync and use the Sync command to create a list. Open the Pocket list.
See the two new links next to the title on each item.
- Basic functionality works--clicking each link opens the expected URL in the browser.
- Test with different screen widths to ensure the title and links break in acceptable positions.
- Test with long titles to ensure the title and links break in acceptable positions. If needed, use the Chrome Dev Tools inside of Obsidian to mock this data (CTRL-SHIFT-I or option-cmd-i for Mac).
- Regression case Ensure the existing alt-click functionality to open the original link when clicking on the item title works.
Add the new {{pocket-url}} variable to the plugin's configured template. Click through the title from one of the Pocket items to see the newly-generated note.
- Ensure the Pocket URL is included in the note and formatted as expected

nybbles · 2022-04-01T17:25:35Z

woohoo! First contribution from the community! ❤️ Thanks for putting this together. I'm reviewing now.

BTW you have to also update version numbers in versions.json.

nybbles · 2022-04-01T17:27:11Z

Also, since (sadly) we do not have automated tests, can you describe how you went about testing this change?

nybbles · 2022-04-01T17:39:21Z

src/ui/components/PocketItem.tsx

+        <PocketItemExternalLink
+          title="Open in Pocket"
+          url={getPocketItemPocketURL(item)}
        />
+        <PocketItemExternalLink title="Open Original" url={item.resolved_url} />


I think it might be clearer to the user to do the following:

Switch the order of these two links, so that the "Open Original comes first".

Rename "Open original" to simply "Open".

So it ends up looking like "Open" and "Open in Pocket", where "Open" is understood to be the existing behavior of opening the saved URL, and "Open in Pocket" is simply a modifier on top of that behavior that opens in Pocket.

LMK what you think.. It seems to me that this would be the easiest to understand for the user.

Sure, I can make those changes.

nybbles · 2022-04-01T17:47:45Z

src/ui/components/PocketItem.tsx

  .item > span {
    display: block;
  }

+  .header {
+    flex-grow: 1


oops. I think a semicolon is missing here.

yup that's the one.. I can build now that I added the semi-colon.

nybbles · 2022-04-01T17:53:59Z

I think it would be a good idea to ensure that when dealing with long titles (or narrow screen sizes), the two open links break onto the next line together, rather than how it is now (the break happens inside of the first link). LMK what you think about that.

ashlyn · 2022-04-01T18:28:39Z

woohoo! First contribution from the community! ❤️ Thanks for putting this together. I'm reviewing now.

BTW you have to also update version numbers in versions.json.

As far as the versions.json file goes--is that just limiting the oldest version of Obsidian this version of the plugin will be compatible with? Is there any reason to change the Obsidian version listed or would listing the following be okay?

  "0.7.1": "0.12.11"

nybbles · 2022-04-01T18:37:55Z

woohoo! First contribution from the community! ❤️ Thanks for putting this together. I'm reviewing now.
BTW you have to also update version numbers in versions.json.

As far as the versions.json file goes--is that just limiting the oldest version of Obsidian this version of the plugin will be compatible with? Is there any reason to change the Obsidian version listed or would listing the following be okay?
  "0.7.1": "0.12.11"

Yup what you posted is sufficient, you don't have to update the Obsidian version numbers. Here's a PR that shows how to bump the version numbers: https://github.com/nybbles/obsidian-pocket/pull/85/files.

nybbles · 2022-04-01T21:10:00Z

Thank you again for this patch!

Address nybbles#70, add template variable pocket-url

945312c

ashlyn changed the title ~~Address #70, add template variable pocket-url~~ Partially address #70, add template variable pocket-url Apr 1, 2022

ashlyn marked this pull request as draft April 1, 2022 15:41

Add links next to title

390d1e9

ashlyn marked this pull request as ready for review April 1, 2022 16:31

nybbles reviewed Apr 1, 2022

View reviewed changes

ashlyn changed the title ~~Partially address #70, add template variable pocket-url~~ Address #70, add template variable pocket-url Apr 1, 2022

Reorder links, fix breaks for narrow screens or long titles

c889318

nybbles closed this Apr 1, 2022

nybbles reopened this Apr 1, 2022

nybbles merged commit 423ad7b into nybbles:master Apr 1, 2022

nybbles mentioned this pull request Apr 1, 2022

Provide the option to open the item directly in Pocket or the original #70

Closed

ashlyn deleted the add-pocket-url-variable branch April 5, 2022 16:48

ashlyn mentioned this pull request Apr 5, 2022

Patch double-opening bug #90

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address #70, add template variable pocket-url #89

Address #70, add template variable pocket-url #89

ashlyn commented Apr 1, 2022 •

edited

Loading

nybbles commented Apr 1, 2022

nybbles commented Apr 1, 2022

nybbles Apr 1, 2022

ashlyn Apr 1, 2022

nybbles Apr 1, 2022

nybbles Apr 1, 2022

nybbles commented Apr 1, 2022

ashlyn commented Apr 1, 2022

nybbles commented Apr 1, 2022 •

edited

Loading

nybbles commented Apr 1, 2022

Address #70, add template variable pocket-url #89

Address #70, add template variable pocket-url #89

Conversation

ashlyn commented Apr 1, 2022 • edited Loading

Relevant Changes

With New Links

Example with Long Titles/Narrow Screen Width

Testing Steps

Basic Running Instructions

Branch Specific Instructions

nybbles commented Apr 1, 2022

nybbles commented Apr 1, 2022

nybbles Apr 1, 2022

Choose a reason for hiding this comment

ashlyn Apr 1, 2022

Choose a reason for hiding this comment

nybbles Apr 1, 2022

Choose a reason for hiding this comment

nybbles Apr 1, 2022

Choose a reason for hiding this comment

nybbles commented Apr 1, 2022

ashlyn commented Apr 1, 2022

nybbles commented Apr 1, 2022 • edited Loading

nybbles commented Apr 1, 2022

ashlyn commented Apr 1, 2022 •

edited

Loading

nybbles commented Apr 1, 2022 •

edited

Loading