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

feat: IPNS Publishing #1973

Merged
merged 36 commits into from
Jul 26, 2022
Merged

feat: IPNS Publishing #1973

merged 36 commits into from
Jul 26, 2022

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Jul 10, 2022

Please see PR #1857 for what happened and background on this. Closes #1482.

Generate New Key

image

Publishing Keys Settings Menu

image

Drop Down

image

Publish

image

image

image

hacdias and others added 30 commits September 20, 2021 15:41
Needs to be eyeballed by a native speaker, but this wins the google search
result count for now.
It was useful in puppeteer, but now we use playwright which
solves waiting logic way better via text selectors
This is basic copy that encourages sharing gateway link + disclaimer
about the need for keeping node online once a day.
Most of fake/guessed progress bars overshoot, as it is better UX to
"finishe sooner than be stuck at 100%"

This change makes expectation always bit bigger than the rolling average
between last two times.
@hacdias hacdias temporarily deployed to Deploy July 10, 2022 07:04 Inactive
@SgtPooki
Copy link
Member

@lidel so something happened with the other PR and I had to create this new one. The E2E tests seem to be failing. I tried the functionality and everything seems to work as expected. Could you take a look at the tests since you were the last working on them (as far as I remember)?

One thing that has broken e2e tests time and time again for me is modifying the package-lock and/or package.json with a different version of node. Be sure you're using node=16.12.0 and npm=8.1.0 and then do:

git checkout origin/main -- package-lock.json

@SgtPooki
Copy link
Member

SgtPooki commented Jul 18, 2022

@lidel so something happened with the other PR and I had to create this new one. The E2E tests seem to be failing. I tried the functionality and everything seems to work as expected. Could you take a look at the tests since you were the last working on them (as far as I remember)?

@hacdias , One thing that has broken e2e tests time and time again for me is modifying the package-lock and/or package.json with a different version of node. Be sure you're using node=16.12.0 and npm=8.1.0 and then do:

git checkout origin/main -- package-lock.json

@hacdias hacdias temporarily deployed to Deploy July 20, 2022 08:53 Inactive
@hacdias
Copy link
Member Author

hacdias commented Jul 20, 2022

@SgtPooki I did that, but there's dependency updates so it changed a bit. Yet, the E2E tests keep failing with:

Requiring @playwright/test second time

I know @lidel worked on the tests of this PR, so it'd be great to get some feedback here!

@lidel lidel self-assigned this Jul 25, 2022
@lidel
Copy link
Member

lidel commented Jul 25, 2022

I finished migration to @playwright/test in #1983, will merge into this PR shortly. 🤞

@lidel lidel temporarily deployed to Deploy July 25, 2022 18:36 Inactive
@lidel lidel removed the status/blocked Unable to be worked further until needs are met label Jul 25, 2022
When someone opens IPNS publishing they now get a hint that more info
and key management can be found in Settings
@lidel lidel temporarily deployed to Deploy July 25, 2022 19:30 Inactive
@lidel lidel temporarily deployed to Deploy July 25, 2022 20:22 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.

Fixed E2E tests, overall lgtm, thank you @hacdias for your patience here ❤️

My hope was to have ipns over pubsub enabled everywhere when we merge this,
but I think having it in Desktop is good enough, and things may improve further
when we have delegated IPNS publishing over Reframe.

Let's wait for 👍 from a native speaker / UX folks till the end of this week,
and then merge.

Copy link
Collaborator

@juliaxbow juliaxbow left a comment

Choose a reason for hiding this comment

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

Alternative text suggestions:

Generate new IPNS Key

  • Previous text: Enter pet name of key to create.
  • New text: Enter a nickname for this key to generate:
  • Description of change: nickname instead of pet name

Drop Down

  • I prefer capitalizing the first letter of each word for dropdowns but not necessary

Publish

  • Previous text: Please wait while initial 20 copies of the updated IPNS record are being stored with the help of DHT peers.

  • New Text: Please wait while the initial 20 copies of the updated IPNS record are stored with the help of DHT peers.

  • Description of change: verb tense

  • Previous Text: Copy the link below and share it with your friends. IPNS address will resolve as long as your node remains available on the network to refresh IPNS record once a day.

  • New text: Copy the link below and share it with your friends. The IPNS address will resolve as long as your node remains available on the network once a day to refresh the IPNS record.

  • Description of change: Added “The” before “IPNS address” ; moved "once a day" to middle of the sentence

  • Question: is sharing with friends the most common use case? Consider replacing “your friends” with “others” or "your network"

@TheDiscordian
Copy link
Member

Just some minor alterations to the text:

"Enter pet name of key to create." -> "Enter a name for the new key."

"Please wait while initial 20 copies of the updated IPNS record are being stored with the help of DHT peers..." -> "Please wait while an initial 20 copies of the updated IPNS record are being stored with the help of DHT peers..."

"IPNS address will resolve as long your node remains available on the network to refresh IPNS record once a day." -> "The IPNS address will resolve as long as your node remains available on the network to refresh the IPNS record once a day."

Thanks for this work, very excited to have this feature in the webUI ^-^.

@hacdias
Copy link
Member Author

hacdias commented Jul 26, 2022

@lidel I updated the copy based on @juliaxbow's and @TheDiscordian's suggestions.

@hacdias hacdias temporarily deployed to Deploy July 26, 2022 07:39 Inactive
@lidel
Copy link
Member

lidel commented Jul 26, 2022

Thank you! Merging so these new labels land in main branch and our translation teams have more time to translate before we cut a new release (~next week?) for Kubo 0.15.

Side notes:

  • context menu is getting crowded
  • feel we could improve presentation of key names and CIDs (right now it is bland monospace font),
    but that requires some additional UX work – let's continue in Improve UX of IPNS publishing #1993

@lidel lidel merged commit 4ff4939 into main Jul 26, 2022
@lidel lidel deleted the feat/1482-settings branch July 26, 2022 12:22
ipfs-gui-bot pushed a commit that referenced this pull request Sep 9, 2022
## [2.18.0](v2.17.3...v2.18.0) (2022-09-09)

 CID `bafybeidb5eryh72zajiokdggzo7yct2d6hhcflncji5im2y5w26uuygdsm`

 ---

### Features

* IPNS Publishing ([#1973](#1973)) ([4ff4939](4ff4939))
* ux improvements to publish modal ([#1998](#1998)) ([ea4f632](ea4f632))

### Bug Fixes

* **ci:** parsing "ipfs-cluster-ctl peers ls" output ([#1966](#1966)) ([828e460](828e460))
* korean detection ([#2005](#2005)) ([362dab8](362dab8))
* Kubo agent text to link via ReleaseLink [#2010](#2010) ([#2011](#2011)) ([3d04988](3d04988))
* nodejs readme badges ([#1985](#1985)) ([a9e661b](a9e661b))
* remove web ui version and keep revision ([#2000](#2000)) ([42ed78d](42ed78d))
* resolve issues with automated releases  ([#1974](#1974)) ([9a7cfad](9a7cfad))

### Trivial Changes

* add missing classnames dependency ([#1977](#1977)) ([c4216b8](c4216b8))
* **deps:** update react-scripts ([#1969](#1969)) ([136b260](136b260))
* pull transifex translations ([#1996](#1996)) ([0de4267](0de4267))
* Pull transifex translations ([#2003](#2003)) ([cc51b15](cc51b15))
* **readme:** NodeJS version support info ([#1986](#1986)) ([829450d](829450d))
* **readme:** update release steps ([#1963](#1963)) ([c5b4822](c5b4822))
* Update .github/workflows/stale.yml [skip ci] ([f15818d](f15818d))
* update storybook and stories ([#2007](#2007)) ([83ceac1](83ceac1)), closes [/github.com/storybookjs/storybook/blob/next/MIGRATION.md#webpack-5](https://github.com/ipfs//github.com/storybookjs/storybook/blob/next/MIGRATION.md/issues/webpack-5)
@ipfs-gui-bot
Copy link
Collaborator

🎉 This PR is included in version 2.18.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Support for publishing to IPNS
6 participants