joshwcomeau / guppy

🐠A friendly application manager and task runner for React.js
ISC License
3.27k stars 154 forks source link

Disable actions that require an internet connection #352

Closed AWolf81 closed 5 years ago

AWolf81 commented 5 years ago

It's more a bug than a feature but I'm writing this as feature description. I've noticed this during my work on PR #335 because there I'm using fetch to get the starters from Github.

Describe the bug With-out internet connection some features aren't working as expected:

To Reproduce

  1. Disable Wifi or disconnect LAN cable
  2. Trigger task that requires a connection:
    • Create a new project & click Let's build
    • Click Add New Dependency
    • Note: Gatsby starter list (new feature in gatsby-starter branch) won't need a special handling as failing silently is OK.
  3. Check console.

Expected behavior Allow tasks that can be performed offline & disable actions that require an internet connection. Also display an infobar that Guppy requires an internet connection on the top of the app (right below the application menu).

Screenshots CRA build with-out internet grafik

Add dependency fails silently grafik

Discussion

actuallyReallyAlex commented 5 years ago

I would like to add that it seems if your network is heavily throttled, the search feature also doesn't work, as described above. Creating a new project shows the animation and takes a long time, but it does eventually work.

Hard for me to tell if this is a proxy issue, a firewall issue, or a slow connection issue.

joshuaellis commented 5 years ago

If no one is looking at this I'm happy to do so.

AWolf81 commented 5 years ago

@joshuaellis sure, it's all yours. If you're having questions or need support during work - please let me know.

I'm pretty busy with writing unit tests. That's why I'm not working on new features at the moment. But maybe I'm doing a break from the tests soon and have a look at the Codesandbox export feature.

joshuaellis commented 5 years ago

UI: Is adding an infobar on top of the screen OK? Should it be dismissable?

A couple of questions relating to the quote, firstly do we already have an infobar designed and built? Not to worry if not. Secondly, I don't think it should be dismissable considering we're going to be blocking core features of the product – building new projects and adding new dependencies.

What're your thoughts?

AWolf81 commented 5 years ago

We don't have an infobar designed. That's a new part of the UI. Yes, you're right. It's OK that it is not dismissable.

joshuaellis commented 5 years ago

I was wondering if we want to have the infobar UI as a separate component that we could use in other instances, is this something we would want or should the Online Checker render an infobar?

AWolf81 commented 5 years ago

I think it's OK to render it in in the OnlineCheck component as this is a special info.

We have had some discussions about notifications (nothing decided) but I think this won't fit into a toastr notification system. We wanted to have some toasts for finishing tasks info or when we'd like to have the users attention on a task - I think there is no open issue for this but I'll check and open an issue so we can discuss this there.

AWolf81 commented 5 years ago

OK, we can close this. Thanks @joshuaellis for adding this.

I like the idea of having a low bandwidth info as @alexlee-dev mentioned. Maybe we could add this as a new issue but I'm not sure if it's worth to add. If you like to see this feature please create a new issue and we can discuss the implementation details there.