Distinct progress bars instead of indeterminate spinners

superhawk610 commented 5 years ago

While installing dependencies, both during step 4 of project creation as well as when adding them manually to an existing project, the UI simply displays an indeterminate spinner until the dependency is installed. To new users, this may be concerning, especially if they are running on older hardware (redux took well over two minutes on my 2011 MBP). NPM's CLI displays a progress bar at the left of its output, so it should be trivial to hook into this and use it to display a more visually appealing distinct progress bar in the UI so users will confidently know that a long-running process is in fact still running and not stalled indefinitely.

I'll work on the logic, but I'm nowhere near @joshwcomeau when it comes to design so any help with styling would be much appreciated.

superhawk610 commented 5 years ago

Installing redux directly (npm i -S redux) took only a few seconds, now to investigate why it took so much longer via Guppy.

superhawk610 commented 5 years ago

NPM CLI's logger uses gauge to display progress bars. They appear differently ~~across different OS's~~ depending on whether or not the terminal reports that it supports Unicode output - below are those that I've observed, let me know if yours is different. They should be fixed at 20 characters wide, including the start- and endcaps.

noUnicode = {
  startCap: `[`,
  complete: ` `, // single space, displayed with white background
  incomplete: `.`,
  endCap: `]`
}

unicode = {
  startCap: `⸨`, // \u2e28
  complete: ` `, // single space, displayed with white background
  incomplete: `░`, // \u2591
  endCap: `⸩` // \u2e29
}

joshwcomeau commented 5 years ago

Hi @superhawk610!

Thanks so much for your involvement in this and other issues :D

Breaking this up into a couple sections:

Dependency installation / updating

Yeah, so this is a real pain point. Dependency installation is really not fun right now.

There are a few thoughts/concerns around this:

I see that you've already commented in #33 (thanks for being so proactive in these discussions!), so you're familiar with the idea of parallelizing them. I think that's a higher priority, since the real pain for me is when I know I need multiple dependencies and I can't just start them all at once.
There's talk about switching package managers in #35. So I think it's worth figuring out which we want to use before spending time trying to parse NPM's output for progress.

I think if we go with the idea in #33, having toast-like notifications (either on top or on the bottom), a progress bar would be really nice. I'd vote for deferring work on this issue until the previous two issues are solved, so that we can really target this solution.

Project creation

I'm not as worried about project creation - it feels more justified that it's slow, since it's a bigger operation. And it's not something done super often (I imagine even serial experimenters still only do it a handful of times a week). That said, it's still not great that the progress bar hangs for quite a while on the "installing dependencies" step.

The good part about this is that the UI is already sorted out; you could just use the progress bar that already exists! And there aren't any dependent issues that I'm aware of, so nothing's stopping you from hacking on this problem. Go for it, if you're interested!

Slower?

Installing redux directly (npm i -S redux) took only a few seconds, now to investigate why it took so much longer via Guppy.

Ah, wild. Guppy uses npm i -SE for dependencies behind-the-scenes, so it should be identical, unless there's a bug with the "ADD_DEPENDENCY_FINISH" action being dispatched too late? Maybe it was just because subsequent installs on the same project are faster?

superhawk610 commented 5 years ago

So after banging my head against this for a couple hours, I'm here to report back. I haven't even gotten around to parsing input yet as so far, I can't seem to generate any. Whether I use childProcess.exec or childProcess.spawn, their stdout and stderr pipes do not generate output until just before the install command finishes.

I'm far from an expert at CLI conventions, but I believe gauge and similar progress reporting solutions (think curl and wget) accomplish this updating single line of information by writing out the information followed by a carriage return (\r) but not a newline, which simply returns the output location to the beginning of the same line and allows it to be overwritten. The problem I'm running into with childProcess is that it doesn't seem to flush output to stdout or stderr until it hits a newline, which as I said earlier doesn't happen until near the end of the process.

I know this is doable because there are full terminal emulators written for Node that correctly flush updates without newlines. Can anyone point me in the right direction?

joshwcomeau commented 5 years ago

(Added a Help wanted label since the last comment ends with a question that I don't know the answer to :D)

superhawk610 commented 5 years ago

Update: headway has been made

enabled: Defaults to true if tty is a TTY, false otherwise. If true the gauge starts enabled. If disabled then all update commands are ignored and no gauge will be printed until you call .enable().

Looks like by default gauge doesn't display the progress bar when its being run through a non-TTY interface, so in order to view npm install progress, we have to mock a TTY. There are a couple packages that accomplish this (pty.js and child_pty), but neither seem to work out of the box with Electron. I'll work on this and update accordingly.

Edit: Ahh, Microsoft has one that appears to be actively supported. This looks promising. Edit 2: Microsoft's is a modernized fork of pty.js, but still has the C++/Python requirements on Windows. Is there any way to bundle these into the production version of Guppy so users wouldn't have to go through making sure requirements are met? Edit 3: Looks like it should bundle everything when building a production Electron build. For Windows users, we'll need to make a note to run

npm install --global --production windows-build-tools

before installing dependencies, otherwise node-gyp will fail if they don't already have the correct Python/C++ installer in their path. Edit 4: IT'S ALIVE 👹

Windows PowerShell                                                              
Copyright (C) Microsoft Corporation. All rights reserved.                       

PS C:\Users\super\code\child-process-test\test-project>                         
PS C:\Users\super\code\child-process-test\test-project> npm install chalk       

[..................] / rollbackFailedOptional: verb npm-session d9ffeb119b9360e 
[..................] \ fetchMetadata: sill resolveWithNewModule chalk@2.4.1 che 
[..................] - fetchMetadata: sill resolveWithNewModule chalk@2.4.1 che 
[..................] | fetchMetadata: sill resolveWithNewModule ansi-styles@3.2 
[..................] \ fetchMetadata: sill resolveWithNewModule color-convert@1 
[      ............] - diffTrees: sill install generateActionsToTake            
[       ...........] / postinstall: sill install executeActions                 
[       ...........] / extract:chalk: verb lock using C:\Users\super\AppData\Ro 
[            ......] / refresh-package-json:chalk: timing action:finalize Compl 
[            ......] - install:chalk: sill doSerial install 56                  
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN test-project@1.0.0 No description                                      
npm WARN test-project@1.0.0 No repository field.                                

[            ......] / postinstall: WARN test-project@1.0.0 No repository field 
+ chalk@2.4.1                                                                   
added 7 packages from 3 contributors and audited 7 packages in 2.018s           
found 0 vulnerabilities

superhawk610 commented 5 years ago

Ok, so now that the proof of concept is done, it's time to decide on UI. As mentioned in #103, we may want to use native notifications for status updates, which do not support any form of live progress display. This leaves us with two options:

Native Solution: Electron supports displaying cross-platform application-wide progress in the form of the dock icon:

Custom Solution: To complement native notifications (which some users may have disabled), we could add a small progress indicator in a corner of the UI that fills as tasks are completed. Quick mock-up:

mockup

These aren't mutually exclusive, they could both be used in conjunction. Thoughts?

Haroenv commented 5 years ago

I think the ideal would be both, and a notification if the app isn’t focused anymore, WDYT?

superhawk610 commented 5 years ago

That complies well with most platform standards, I like it. I think Electron also has a Mac-only API for the dock icon to show a numeric badge, like the notification badge on apps for missed notifications, we could probably integrate with that as well if a task finishes while the application is in the background.


const dock = electron.app.dock;
if (!dock || window.isFocused()) return; // ignore if in foreground or on Windows

dock.setBadge(notificationCount); // I belive this takes a string so text works as well

joshwcomeau commented 5 years ago

Thanks a ton for looking into this =D

Ah, I love the idea of dock icon progress! Brilliant idea.

I like the mockup you proposed, for also having an in-app indicator; I think I'd opt for something smaller / more subtle, but I like the idea.

How do you envision either of these ideas working with queued items? Say I'm currently installing 1 dependency, but there's a clustered group of 3 dependencies next, followed by removing a dependency. I think we could probably treat them as discrete events; The progress bar represents that first dependency. When it finishes, the progress resets to 0 and starts climbing again. It might be a bit surprising for folks, but I think the alternative is too unpredictable to provide realistic progress (plus it'd be weird if the progress bar suddenly jumped backwards a bunch when adding new stuff to the queue). Also, it seems simplest, and given the inherent complexity in a queue, I like the idea of going for the simplest approach.

For notification badges... I'm not sure I like the idea. For me, a notification badge should represent an unexpected update (eg. you received a message, the app wants to update). I don't often see them used for predicted things (eg. download complete). I think it might be a bit underwhelming if you see a red "2" on the icon, and clicking over just reveals that the thing you expected had happened. Plus we'd need some additional UI to make it clear what the badge represents, otherwise it might just be confusing.

Haroenv commented 5 years ago

I think Transmission shows a red bubble of a download is done, but only when app isn’t focused. (As an example of the usage)

j-f1 commented 5 years ago

You could use something like are-we-there-yet to manage the progress of several tasks.

superhawk610 commented 5 years ago

@Haroenv yeah Transmission was what I had in mind too haha.

@j-f1 our task queue is essentially synchronous since no more than one action can run on a single project at a time and tasks run one after the other. I imagine the progress indicator would be distinct per project and largely handled by the reducer. I'll keep that in mind though if we ever do manage to incorporate more async parallel tasks.

j-f1 commented 5 years ago

It would still be useful for a global progress bar since multiple projects could be installing stuff at the same time.

joshwcomeau commented 5 years ago

You could use something like are-we-there-yet to manage the progress of several tasks.

Redux saga comes with some pretty fancy control-flow tools... I think I'd like to see if we can do it with the existing tools before reaching for another, although definitely not against it if there's a pain point it can address!

AWolf81 commented 5 years ago

@joshwcomeau You're right, adding the progress or a number to the dock icon could be difficult to understand. I'm only seeing the number badge for e-mail count or notifications and not for as you said something that you're expecting. (Just Windows mail is using a badge on my machine.)

And I would also calculate the progress for the current task and not for all active tasks - easier to implement and the user experience is OK.

Maybe the progress bar could replace the pixelated guppy (screenshot below). I like the draft of @superhawk610 but not in the top right corner. I think it should be at the dependency panel. loading guppy

superhawk610 commented 5 years ago

@AWolf81 the only issue with having it at the dependency panel is that during batched installs, we only have one progress bar for multiple dependencies, so if you're installing 3 dependencies, all their respective progress bars would move together - not terrible, but it feels a bit disingenuous, I guess?

I also really like the pixel guppy, so there's that 😁

AWolf81 commented 5 years ago

@superhawk610 Ah OK, so we have to keep the guppy as indetermined spinner or do a special handling for the progress of batch installs. e.g. the overall bar shows 1 of 3 installations finished.

Maybe we can add the loader close to each dependency and replace that spinner - so it's possible to show a spinner for each dependency.

joshwcomeau commented 5 years ago

I also really like the pixel guppy, so there's that 😁

This was originally supposed to be the logo! But it looked too much like a spinner, plus I decided pixel-art wasn't a good fit for the rest of the design.

(I actually commissioned 3 separate logos, before @superhawk610 and @AWolf81 both contributed better ones :D although I bought them on fiverr, so it was quite inexpensive).

For finding an area for dependency progress, hm. I like @AWolf81's suggestion of adding an indicator to each dependency in the list, could be a radial progress bar as suggested earlier.

BTW, we use radial progress bars at Khan Academy - we did it by using an SVG circle, giving it a thick strokeWidth, setting the stroke-dasharray equal to the length of the perimeter, and then animating the stroke-dashoffset to "move" a stroke through it. Not sure how you were planning on tackling this, @superhawk610, but just in case you weren't sure, I know this strategy has worked for us :)

joshwcomeau / guppy