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

WIP: Feat/backend toggle switch for desktop settings #895

Conversation

phoniks
Copy link

@phoniks phoniks commented Dec 4, 2018

Seeking feedback. I'm working on enabling the user to try out the Javascript implementation of IPFS by toggling a switch on the settings page. This PR simply ads a checkbox on the DesktopSettings page. I'm particularly interested in knowing whether having this as a checkbox makes sense. Originally I had in mind, based on the original discussions of this feature, to make this setting a select component, but during coding it became obvious that doing so would add a lot of code for not very much change in user experience. Perhaps in the future, if there are other implementations that can be chosen, it may make sense to do the dropdown, but for now it doesn't seem prudent. But, perhaps since it potentially takes your node offline there should be an intermediate step between flipping the switch and actually shutting down the node and re-spawning. Maybe a quick confirmation alert?

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.

  • I think a confirmation dialog asking if user want to restart ipfs-desktop with the new backend makes sense (with two options: Yes / No) 👍
  • The added value of having a select is that it gives us a venue to display backend version:
    • go-ipfs v0.4.18
    • js-ipfs v0.33.1
      Not sure how useful it is, but my personal preference would be to have a select. With checkbox user does really know what is being selected if "Javascript" is not checked.

@olizilla thoughts?

@jessicaschilling
Copy link
Contributor

@phoniks - please accept sincere apologies for this PR getting lost in the fray. Is this something you'd still like to discuss?

@phoniks
Copy link
Author

phoniks commented Apr 9, 2020

If it's still something that'd be valuable to the project, I'm happy to follow up on adding the functionality.  The last time I had checked in on this, I think some things about the project had changed pretty significantly, so it was going to require some additional work that I didn't have the bandwidth for at the time. I put it on the back burner and kind of lost track of it, but since we're all on mandatory Quarantine and Chill mode right now I could take another look.

@jessicaschilling
Copy link
Contributor

We're ramping back up on GUI-related work for this quarter, so any work is welcome -- though if you'd be interested in taking a look at the current state of the landscape while you quarantine and chill, have a look at https://github.com/ipfs/ipfs-gui/blob/update-readme/README.md (as you can see, it's in a PR so not final yet, ha!). There's info there about the general methodology we're taking, and a link to a unified ZenHub board for a glimpse into current open needs ... so there might be something there that you're more interested in these days? Happy to chat further if anything catches your eye. 😊

@lidel
Copy link
Member

lidel commented Apr 27, 2020

Thank you for bringing this up @phoniks!

Since this PR was opened we decided to decouple ipfs-desktop-specific settings from ipfs-webui (ipfs/ipfs-desktop#1389, #1445) and move them to a submenu (ipfs/ipfs-desktop#1425):


That is why I am closing this PR, but the idea of giving a choice to switch to js-ipfs is still valid and is tracked under ipfs/ipfs-desktop#601

Going forward ipfs-webui will not care about backend type, as long as it conforms to the CORE API spec.

@lidel lidel closed this Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants