Adding visual indication a commit is in progress

[Xapphire13]

Fixes #929

I was thinking about adding integration with busy-signal (and may still do in the future), but in my testing, the busy signal is far too discrete for this kind of thing; unless you're looking for it, you will still be in that state of "is the commit happening?"

Instead, my solution it to change the text of the commit button to Working.... I believe this to be highly noticable (since the user's focus was just there), and non-intrusive (as opposed to opening a progress indicator or toast notification, which would yield a UI that jumps around hiding/showing elements).

[winstliu]

Suggestion: why not Committing... instead?

[Xapphire13]

I guess because it's not strictly only committing, the longest delays can be the pre-commit hook. So I chose a word that seemed to fit all interpretations

[BinaryMuse]

@kuychaco is doing some work in #1230 to add some state to the Repository model to indicate when certain operations (including committing) are in progress. We'll probably wait until that's merged and then make the appropriate changes here.

As far as a UX, I'd like to come up with something that helps prevent the user from putting the UI in a bad state. I'll comment more on this in the near future.

[Xapphire13]

Oh cool! I just went through the PR and it seems that (at least currently) my PR won't conflict with that work. I only modify the view, which would automatically switch to getting the state from the controller in @kuychaco's PR

[Xapphire13]

@kuychaco, @BinaryMuse

Is it worth me keeping this PR open, or should I close it?

[BinaryMuse]

@Xapphire13 I've merged master into this branch; if it goes green, I'm gonna merge it, and we can address any additional UI stuff we want to do here later, unless @kuychaco has any 💭 s otherwise.

[kuychaco]

👍 thanks for the PR @Xapphire13. I'm cool with merging. One concern - @BinaryMuse and I discussed adding a delay before changing the commit button text to prevent flickering that is distracting in the case of quick commit operations (see research about response time limits). We can do this work in a separate PR, unless you wanna take a stab at it

[Xapphire13]

@kuychaco, sounds good. I gave the document a skim and it seems that this should have a 1 second delay (or there abouts). What would you recommend in regards to triggering the rerender for that? Internal state or another prop that is passed in?

[Xapphire13]

@kuychaco @BinaryMuse

I have updated the PR to include a 1 second timer. I implemented it using a method running in a closure.

[kuychaco]

Great work @Xapphire13! Thanks for your contribution ✨

[Xapphire13]

Thanks for the merge! And for the info on response times =]