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

fix: local and public gateway links when cannot preview #1834

Merged
merged 7 commits into from
Sep 1, 2021
Merged

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Aug 14, 2021

Fixes #1767 by implementing the suggested solution: link to both local gateway and to ipfs.io.

Please note that availableGatewayUrl can be either the local gateway or ipfs.io and that is still used for all other types.

I could not reproduce #1767 since my computer always got the correct local gateway URL as the available gateway. To show both links, we hard code the ipfs.io link and fetch the actual localGatewayUrl from settings.

image

This PR superseeds #1826.

License: MIT
Signed-off-by: Henrique Dias hacdias@gmail.com

License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
@hacdias hacdias marked this pull request as ready for review August 14, 2021 08:21
@hacdias hacdias removed the request for review from rafaelramalho19 August 14, 2021 08:21
@hacdias hacdias temporarily deployed to Deploy August 14, 2021 08:26 Inactive
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Looks good, but we should remove hardcoded ipfs.io and enable users to use gateways of their choosing.

  1. rename "with ipfs.io" to "public gateway"
  2. hide "local gateway" if it is same as public one
  3. make public gateway customizable
    • add "Public Gateway" section to "Settings" screen (you can reuse "API" section for look and feel)
    • it should read value from window.localStorage.get('ipfsPublicGateway') and if missing fallback to showing https://dweb.link
    • replace ipfs.io hardcoded in the app with value from window.localStorage (with fallback to https://dweb.link)

@hacdias
Copy link
Member Author

hacdias commented Aug 17, 2021

If Public Gateway === Available Gateway (local)

Screen Shot 2021-08-17 at 17 29 33

Otherwise

Screen Shot 2021-08-17 at 17 29 41

Public Gateway Setting

Screen Shot 2021-08-17 at 17 29 47

If bad http url

Screen Shot 2021-08-17 at 17 29 54

If empty

Screen Shot 2021-08-17 at 17 30 00

Default: https://dweb.link

@hacdias hacdias temporarily deployed to Deploy August 17, 2021 15:36 Inactive
@hacdias hacdias requested a review from lidel August 17, 2021 15:41
@hacdias
Copy link
Member Author

hacdias commented Aug 17, 2021

Updated @lidel

@hacdias hacdias changed the title fix: local gateway and ipfs.io when cannot preview fix: local and public gateway links when cannot preview Aug 17, 2021
@hacdias hacdias self-assigned this Aug 18, 2021
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Looks sweet!

I'm excited for this one – we will leverage the window.localStorage trick in brave and ipfs-companion, to ensure webui uses gateway user set in respective GUI 😻

Needs review

Needs fixing:

License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
@hacdias
Copy link
Member Author

hacdias commented Aug 19, 2021

@lidel both fixed. The first point "Opening non-unixfs DAG like" is translation related (see e12efec). I only updated for English as I'm assuming everything else needs to go through Transifex.

@hacdias hacdias temporarily deployed to Deploy August 19, 2021 07:45 Inactive
@hacdias hacdias requested a review from lidel August 23, 2021 11:06
cosmetic fix that adds reset button to public gateway section in
Settings and disables buttons when they are no-op.
shareable links are better way to sonvey the practical meaning of this
setting, other places are incidental
@lidel lidel temporarily deployed to Deploy September 1, 2021 19:25 Inactive
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thank you!

Tweaked UI a bit so buttons are active only when they actually do something:

Screenshot 2021-09-01 at 21-33-02 Settings IPFS

Merging + waiting with release till Friday for new labels to appear on Transifex.

FYSA filled follow-up issues:

@lidel lidel merged commit 4bf78a4 into main Sep 1, 2021
@lidel lidel deleted the fix/1767 branch September 1, 2021 19:41
@autonome
Copy link

autonome commented Sep 1, 2021

bah just saw this - the language and changes look good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

The private network browsing file references the ipfs.io gateway
3 participants