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(settings): randomize pinning service templates #2027

Merged
merged 2 commits into from
Sep 19, 2022

Conversation

lidel
Copy link
Member

@lidel lidel commented Sep 15, 2022

Now that we have 4+ services (#2026) we should always show them in random order, so we do not promote any specific one above others.

This PR:

@lidel lidel requested a review from a team as a code owner September 15, 2022 19:47
@lidel lidel temporarily deployed to Deploy September 15, 2022 19:54 Inactive
@SgtPooki
Copy link
Member

@lidel We received feedback during IPFS camp that the randomization of things on the awesome-ipfs website (tracked in the discuss linked in ipfs/awesome-ipfs#420) that the randomization there is more harmful than it is helpful. I think the randomization here would also do the same.

There is an argument to be made for not promoting one over the other, but I think UI consistency is more important. Something I would like to see before doing any sort of randomization would be removing services that you've already added. For example, I already have Pinata added to my pinning services, with API_TOKEN, but it still shows in the list of providers to add.

Users may want to add a provider more than once, but should those providers be in the same spot as they were when the user last added that provider? Should we have a separate section for the providers that have already been added?

I am not a fan of this change and would rather see a more simple & elegant solution that keeps a consistent UI.

@@ -33,21 +35,21 @@ const pinningServiceTemplates = [
{
name: 'Web3.Storage',
icon: 'https://dweb.link/ipfs/bafybeiaqsdwuwemchbofzok4cq7cuvotfs6bgickxdqr6f7hdt7a64cwwa/Web3.Storage-logo.svg',
apiEndpoint: 'https://api.web3.storage/pins',
apiEndpoint: 'https://api.web3.storage',
Copy link
Member

Choose a reason for hiding this comment

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

oops.

@lidel
Copy link
Member Author

lidel commented Sep 15, 2022

There is an argument to be made for not promoting one over the other, but I think UI consistency is more important.

This randomization happens on page load, and not every time on modal open.
This means the user will see the same order when they add multiple services and open the modal multiple times.

@SgtPooki lmk if you feel this is still not enough. Alternatively, I could sort them in a deterministic way, e.g., based on PeerID.
This way every user will always get the same order, but it will be randomized across users, so no service will be promoted as the default (first).

[..] removing services that you've already added. For example, I already have Pinata added to my pinning services, with API_TOKEN, but it still shows in the list of providers to add.

This is bad UX: user should be able to use multiple tokens against the same endpoint. Having "Recently used" section at the top sgtm.

@SgtPooki
Copy link
Member

I'm good with randomize on module load.. but random on page load would result in changing order any time they go back to the modal/settings page, right...?

@SgtPooki SgtPooki merged commit cd211ac into main Sep 19, 2022
@SgtPooki SgtPooki deleted the fix/randomize-pinning-services branch September 19, 2022 16:52
ipfs-gui-bot pushed a commit that referenced this pull request Oct 3, 2022
## [2.19.0](v2.18.1...v2.19.0) (2022-10-03)

 CID `bafybeiavrvt53fks6u32n5p2morgblcmck4bh4ymf4rrwu7ah5zsykmqqa`

 ---

### Features

* add codecov_token to codecov step ([#2044](#2044)) ([ee7cc63](ee7cc63))
* add Web3.Storage and Estuary to pinning service templates ([#2026](#2026)) ([3aae1f2](3aae1f2))

### Bug Fixes

* **settings:** randomize pinning service templates ([#2027](#2027)) ([cd211ac](cd211ac))

### Tests

* add unit&storybook coverage to codecov ([#2023](#2023)) ([1961b17](1961b17))

### Trivial Changes

* go-ipfs -> kubo name update ([#2022](#2022)) ([183c476](183c476))
* pull new translations ([#2041](#2041)) ([1be6342](1be6342))
* Pull transifex translations ([#2028](#2028)) ([5011e12](5011e12))
* replace topojson with topojson-client ([#2035](#2035)) ([e06a878](e06a878))
@ipfs-gui-bot
Copy link
Collaborator

🎉 This PR is included in version 2.19.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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants