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

Pinning service integration: Augment "Pin IPFS Resource" functionality #888

Closed
2 tasks
jessicaschilling opened this issue May 28, 2020 · 13 comments · Fixed by #997
Closed
2 tasks

Pinning service integration: Augment "Pin IPFS Resource" functionality #888

jessicaschilling opened this issue May 28, 2020 · 13 comments · Fixed by #997
Assignees
Labels
area/pinning Integrating pinning into GUI apps effort/days Estimated to take multiple days, but less than a week exp/intermediate Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature P0 Critical: Tackled by core team ASAP status/deferred Conscious decision to pause or backlog topic/design-front-end Front-end implementation of UX/UI work topic/design-ux UX strategy, research, not solely visual design topic/design-visual Visual design ONLY, not part of a larger UX effort
Milestone

Comments

@jessicaschilling
Copy link
Contributor

jessicaschilling commented May 28, 2020

Note: This issue is part of a larger pinning service integration epic undertaken spring/summer 2020.

The need

Augment functionality of existing "Pin IPFS Resource" toggle to support the use of third-party pinning services:

  • Keep UI of existing "Pin IPFS Resource" toggle as-is (don't break UX for people who are used to "pinning via Companion")
  • When a resource is not pinned, and the toggle is clicked, in addition to creating low-level pin (current behavior), it also imports to MFS and opens imported resource in the same UI as "Quick Import" (e.g. takes you to appropriate page in Files screen)

Context

Advantages of this approach are:

  • works the same as importing non-IPFS data, reducing cognitive overhead
  • they can immediately manage imported resource via UI on webui's Files screen
  • works fine even if user pins entire wikipedia (lazy-loading, only pages they actually visited are kept around)

Original note from PRD:

(TBD) Feature in IPFS Companion that allows a user to pin data to their chosen pinning service

  • This is likely unnecessary with auto-upload rules for pinned files

User flow:

  • Users can use Desktop or WebUI to connect their local node to an IPFS pinning service
  • User right-clicks a file or image and selects “pin to IPFS”
  • If they have an auto-upload rule configured, that file is automatically added to their associated pinning service
@jessicaschilling jessicaschilling self-assigned this May 28, 2020
@jessicaschilling jessicaschilling added the area/pinning Integrating pinning into GUI apps label May 28, 2020
@jessicaschilling jessicaschilling added kind/enhancement A net-new feature or improvement to an existing feature P0 Critical: Tackled by core team ASAP labels May 28, 2020
@jessicaschilling jessicaschilling added exp/intermediate Prior experience is likely helpful effort/days Estimated to take multiple days, but less than a week status/in-progress In progress topic/design-front-end Front-end implementation of UX/UI work topic/design-ux UX strategy, research, not solely visual design topic/design-visual Visual design ONLY, not part of a larger UX effort labels May 28, 2020
@jessicaschilling
Copy link
Contributor Author

@lidel and @rafaelramalho19 --

AFAIK, there are two scenarios in which a Companion user might invoke third-party pinning. Both only occur if the user has "Auto upload" enabled for one or more pinning services.

  1. From the "Pin IPFS resource" toggle in the Companion drop-down.
    image

  2. When right-clicking on a page/image and selecting "Import to IPFS".
    image

This seems logical enough that it doesn't need any modification or explanation, but want to run that past you both to make sure (a) that I haven't omitted any possible scenarios (b) it doesn't feel logical/obvious to you. Thanks!

@lidel
Copy link
Member

lidel commented Jun 1, 2020

  • 👍 Flow for (2) "Import to IPFS" is already ending in Web UI, so I don't think we need to do anything there.

  • ❓ Depending on how pinning service integration is implemented, "pinning current URL" (1) may be problematic. If "auto pinning" requires webui running to work, then the existing toggle won't trigger pinning at remote service. We may rewire this toggle to open "pinning screen" in webui instead.

@jessicaschilling
Copy link
Contributor Author

@lidel Sounds good. Let's keep this on hold until we know a bit more about the issues involved in (1).

@jessicaschilling jessicaschilling added status/deferred Conscious decision to pause or backlog and removed status/in-progress In progress labels Jun 1, 2020
@jessicaschilling
Copy link
Contributor Author

@lidel - Revisiting your earlier question for a status check:

❓ Depending on how pinning service integration is implemented, "pinning current URL" (1) may be problematic. If "auto pinning" requires webui running to work, then the existing toggle won't trigger pinning at remote service. We may rewire this toggle to open "pinning screen" in webui instead.

@lidel
Copy link
Member

lidel commented Sep 9, 2020

Coming up with good "pinning UI" in Companion will be difficult because exposing user to the concept of a "pin" itself is feels too technical IMO.

Perhaps a better user experience would be if we replace "Pin IPFS Resource" with "Import to my IPFS node" which... copies reference to the current CID to MFS, and then opens Files in WebUI in a new tab (just like wealready do when user does quick upload or import via context menu).

This approach means we don't need any new UI in Companion, and lean more on WebUI, which is our general direction anyway.

Note that import to MFS effectively "pins" content that is already in node's cache (as long it is in MFS, it won't be garbage-collected), but is much easier for user to reason about:

  • works the same as importing non-IPFS data, reducing cognitive overhead
  • they can immediately manage imported resource via UI on webui's Files screen
  • no need to learn what "pinning" is, but if they wish, they can pin it locally/remotely via webui
  • works fine even if user pins entire wikipedia (lazy-loading, only pages they actually visited are kept around)

@jessicaschilling how does this sound?

@lidel lidel added this to the v2.16 milestone Sep 9, 2020
@lidel
Copy link
Member

lidel commented Sep 9, 2020

Refining the idea from my previous comment:

  • keep existing "Pin IPFS Resource" toggle as-is (dont break UX for people who are used to "pinning via companion")
  • when resource is not pinned, and the toggle is clicked, in addition to creating low level pin, it also imports to MFS and opens imported resource in the same UI as "Quick Import"
    • this enables all the good things I mentioned in previous comment, without any regression for existing users
  • webui's Files: pin indicator is finally useful: can be used as entry point for checking local pin status and managing remote pins (when Epic: Pinning service integration ipfs-gui#91 lands)

@jessicaschilling
Copy link
Contributor Author

@lidel - sounds excellent, totally agree. I'm going to repurpose this issue from discussion to requirements now that we have those. This should wrap up pinning-related Companion work, at least for initial release 😊

@jessicaschilling jessicaschilling changed the title Pinning integration: Determine flow for Companion Pinning service integration: Augment "Pin IPFS Resource" functionality Sep 9, 2020
@jessicaschilling
Copy link
Contributor Author

Two thoughts below on i18n keys stashed from #937 but not implemented to save on contributor translation burden. Let's use this (or similar) wording: "assets" is a little clearer than "resources".

"panel_pinCurrentIpfsAddress": { "message": "Pin IPFS Assets", "description": "A menu item in Browser Action pop-up (panel_pinCurrentIpfsAddress)" },
and
"panel_pinCurrentIpfsAddressTooltip": { "message": "Pin the IPFS assets on this tab (e.g. files, images, videos) to your node to have local copies that are available offline and never thrown away.", "description": "A menu item tooltip in Browser Action pop-up (panel_pinCurrentIpfsAddressTooltip)" },

@lidel
Copy link
Member

lidel commented Nov 2, 2020

Small concern regarding low level pins:

  • If we keep the toggle, creating low level pin by default will remain a problem:
    • someone may want to click the toggle on /ipns/en.wikipedia-on-ipfs.org/wiki/ and that will cause 650GB download
    • ideally, we would inform user about the size of data about to be fetched&pinned

What if we don't do low level local pinning in Companion, but delegate that decision to Web UI?

  • Idea: clicking on the toggle would copy the current content path to MFS and open it in webui's Files screen + automatically open the Set pinning modal for newly copied item
    • we could do this easily by adding special route /pinning or something, which would be alias for /files + Set pinning modal
    • afaik copying content path to MFS without low level pin will not trigger download of the entire thing, but will protect subset of data that ends up in local cache
      • in this case only visited wikipedia pages will be kept around, not the entire thing, which is a very powerful abstraction
    • Set pinning modal in Files
      • could display total size of data that is about to be pinned, making pinning more informed, and give user ability to pin it remotely instead of locally
      • challenge here is to make it clear that "local pin" will fetch entire thing vs "just keeping it in MFS" acts as a "lazy pin"
        • perhaps renaming "Cancel" button in Set pinning modal to something else that indicates "don't pin everything, but keep already cached data around as long its present in My Files (MFS)" ?
        • or maybe renaming "Local pin" to more explicit "Fetch everything and pin locally" ?
        • finding out a clear and concise UI and language for pinning interaction from Companion is a challenge :(
          • @jessicaschilling would like to give it a try it in Figma or something? If we solve flow for pinning /ipns/en.wikipedia-on-ipfs.org/wiki/, we will solve it for everything else.

@jessicaschilling
Copy link
Contributor Author

@lidel I think this might actually be a slightly different problem from the perspective of the end user. Unless you know pretty deeply how pinning works, the default user mindset is likely to be something like this:

  • Goes to, for example, the Queen Victoria Wikipedia-on-IPFS page
  • Thinks "I would like this page offline" and pins it
  • "Accidentally" pins the entirety of Wikipedia

Could we give the user the option to "pin this page" (which would, if I understand correctly, take the user to the Files screen + modal to decide if they want to MFS-pin just the local cache to their node or a pinning service) versus some more nuclear "pin entire site" option (which would have to come with warnings: you're pinning 80gb)?

In either case, agree we shouldn't be doing low-level pinning from Companion anyway, because your pins "disappear" and can never be seen from our file system for the ordinary user.

@jessicaschilling
Copy link
Contributor Author

Continuing forward, notes from call today. @lidel - please amend/comment if I've got anything incorrect or you have additional thoughts.

  1. We're already framing all those menu items under the context of "Current Tab", so it's implicit we're only talking about the page in question (this is also an existing problem: who knows how many folks have accidentally pinned something massive thinking they were only acting on one page).

    • result: Let's make sure action in menu only applies to current tab, not entire surrounding site
    • We can introduce "do this for entire site" functionality later if there is a user demand
  2. Given the possible addition of multiple pinning services for the user, PLUS existing confusion of "real pin/low-level pin" vs "pseudo-pin/MFS import", it makes the most sense to keep the actual pinning activity happening entirely in Web UI vs being triggered from Companion. Proposed flow:

    • Replace existing pin toggle menu item with an item Keep Offline Copy ...; helptext would be something like Import all IPFS assets on this tab (e.g. files, images, videos) to your node to have a local copy that's available offline and never thrown away.
    • Clicking this item imports the local cache of Queen_Victoria.html into directory /ipfs-companion-imports/en.wikipedia-on-ipfs.org/wiki/ or similar
    • User is then taken to the directory page on Web UI, so they can see what just happened, and potentially pin either locally or to a third-party service
    • Unless user moves the location of that import directory, clicking Keep Offline Copy ... of other ipfspedia pages would result in them being saved in a directory structure that matches that of the original site URL

NOTE: this does result in low-level pinning being removed from Companion's overall functionality. Are we OK with this? (Suggesting yes, because "I don't understand why my repo is 100GB and GC doesn't change that" is a common new-user complaint and is likely largely triggered by accidental massive low-level pinning. PLUS it removes the need to explain "real pin/low-level pin" vs "pseudo-pin/MFS import" - something fairly nuanced - to early-stage who may not need to understand this.)

@lidel
Copy link
Member

lidel commented Nov 3, 2020

I agree:

  • As illustrated by DNSLink website pinning failed #941, low level pins remain a serious UX problem, and we should just remove them from Companion and control UX in WebUI.
    • There would be no toggle, just Keep Offline Copy ... (name TBD) that imports current tab to MFS and opens it in Files

Devil is in the details:

  • what is actually stored in Files?
    • only the root document behind the URL in address bar?
      • this is fine for image, video, pdf, but tricky if document is HTML page with subresources
    • the root document + its IPFS assets?
      • we can't change asset URLs in HTML, because that would change the CID of cached document, but we could just cache assets under URL-based paths to ensure our node keeps them around (a pragmatic compromise)
  • I think we need to use separate directory, eg. /ipfs-companion-shapshots/{domain}/paths/to/page because /ipfs-companion-imports is time-based, and here we want to maintain caches per specific URL
    • perhaps menu item should be less technical, eg. Copy (Snapshot?) to My Files.. ?
    • what happens when we perform action for URL which was already stored before?
      silently overwrite to the latest version?

@jessicaschilling
Copy link
Contributor Author

we can't change asset URLs in HTML, because that would change the CID of cached document, but we could just cache assets under URL-based paths to ensure our node keeps them around (a pragmatic compromise)

This sounds good. Additionally, "capture this web page" is a known feature request, so definitely worth the effort 😊

I think we need to use separate directory

Makes sense. What about Save Page Snapshot... for text?

I'd advise against silently overwriting, since one use case might be archival saves in (for example) a censorship situation where changes over time are important to document.

  • A dialog with options to overwrite vs date-append in some way would take care of this, but would that live in the context of Companion, or Web UI?
  • How would date-appending work in the context of the directory? '/ipfs-companion-shapshots/{domain}-DATE/paths/to/page'?

@lidel lidel modified the milestones: v2.16, v2.17 Nov 18, 2020
@lidel lidel modified the milestones: v2.17, v2.18 Jan 11, 2021
lidel added a commit that referenced this issue May 7, 2021
This changes the way IPFS content is kept around.
Instead of using low level pins we copy the item to MFS,
unifying experience to match recent changes in ipfs-webui v2.12
and ipfs-desktop v0.15.0

This also removed page-action, because it was Firefox-specific feature
and made maintenance and testing more difficult
(now we have same UX in all browsers).

Closes #742
Closes #888
Closes ipfs/ipfs-gui#91
lidel added a commit that referenced this issue May 10, 2021
This changes the way IPFS content is kept around.
Instead of using low level pins we copy the item to MFS,
unifying experience to match recent changes in ipfs-webui v2.12:
https://github.com/ipfs/ipfs-webui/releases/v2.12.0
and ipfs-desktop v0.15.0:
https://github.com/ipfs/ipfs-desktop/releases/tag/v0.15.0

This also removed page-action, because it was Firefox-specific feature
and made maintenance and testing more difficult
(now we have same UX in all browsers).

Closes #742
Closes #888
Closes ipfs/ipfs-gui#91

Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/pinning Integrating pinning into GUI apps effort/days Estimated to take multiple days, but less than a week exp/intermediate Prior experience is likely helpful kind/enhancement A net-new feature or improvement to an existing feature P0 Critical: Tackled by core team ASAP status/deferred Conscious decision to pause or backlog topic/design-front-end Front-end implementation of UX/UI work topic/design-ux UX strategy, research, not solely visual design topic/design-visual Visual design ONLY, not part of a larger UX effort
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants