Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Comments

Indicate when a package is updating in the status bar tile#884

Merged
winstliu merged 12 commits intomasterfrom
wl-super-dynamic-updates-tooltip
Dec 10, 2016
Merged

Indicate when a package is updating in the status bar tile#884
winstliu merged 12 commits intomasterfrom
wl-super-dynamic-updates-tooltip

Conversation

@winstliu
Copy link
Contributor

@winstliu winstliu commented Dec 7, 2016

Closes #883

Some things to note:

  • The tile changes have been extracted into a helper updateTile method which I think helps streamline and deduplicate things (see below).
  • This PR fixes a bug in my previous PR where I accidentally set the priority of the tile to 0 when reinitializing it instead of -99. The new helper method takes care of this.
  • This PR also fixes a different bug in my previous PR where we were trying to call tooltip.dispose() on a non-existent tooltip if the Check for Updates button was pressed. Whoops!
  • I fixed a bug in PackageManager where the package-updating event was being generically emitted through Emitter rather than through the dedicated emitPackageEvent helper method. This means that theme-updating will now start to be emitted for themes.

TODO:

  • Finish what to display for failed updates
  • Get some feedback on the label text
  • Specs

Also, remove the failed update from the array if a subsequent try
successfully updates it.
@winstliu
Copy link
Contributor Author

winstliu commented Dec 8, 2016

Thoughts: should the x updates text include the number of packages that are currently updating? For example, if there are 5 packages and 3 are updating, should the text read 5 updates (3 updating), or 2 updates (3 updating)? Or maybe 3/5 updating, 3 out of 5 updating?

I think I like the last two the most.

In addition, regarding failed updates, should we remove the failed text when the tile is clicked? And should we remove that package from the available updates count?

/cc @atom/feedback

@damieng
Copy link
Contributor

damieng commented Dec 8, 2016

I'd like "5 package updates available" and once the process starts "5 packages updating" then "4 packages updating"... until gone. i.e. instead of 3-5 just count down from 5.

@simurai
Copy link
Contributor

simurai commented Dec 8, 2016

@damieng once the process starts "5 packages updating"

I like that too because it's simpler. But what if there a 5 packages in total and you only click the update button for 3 packages. In that case "5 packages updating" sounds wrong.

Hmm.. maybe still write it out? It's longer but also clearer. The above example would be:

  • 5 package updates available
  • 5 package updates available (3 updating)
  • 4 package updates available (2 updating)
  • 3 package updates available (1 updating)
  • 2 package updates available

In addition, regarding failed updates, should we remove the failed text when the tile is clicked? And should we remove that package from the available updates count?

Maybe keep it as long as the settings-view shows an update as failed. I guess if you reload Atom it will show again as "normal" update.

  • 5 package updates available
  • 5 package updates available (3 updating)
  • 4 package updates available (2 updating)
  • 4 package updates available (1 updating, 1 failed)
  • 3 package updates available (1 failed)

Actually, is this only for the tooltip? Or will the status-bar tile also show if updates are in progress?

@winstliu
Copy link
Contributor Author

winstliu commented Dec 8, 2016

This is just for the text displayed in the status bar. The tooltip is more verbose and is already basically what you're suggesting: 5 package updates available, 3 currently updating. That's why I suggested for the text to be more succinct.

@simurai
Copy link
Contributor

simurai commented Dec 8, 2016

Ok, for the status-bar I like your 3/5 updating suggestion. Same example from above 5 total only 3 get clicked:

  • 5 updates
  • 3/5 updating
  • 2/4 updating
  • 1/3 updating
  • 2 updates

And when failing:

  • 5 updates
  • 3/5 updating
  • 2/4 updating
  • 1/4 updating (1 failed)
  • 3 updates (1 failed)

@winstliu
Copy link
Contributor Author

winstliu commented Dec 9, 2016

Ok I think this is ready. The only thing that might still have room for improvement is the failed text.

@lee-dohm
Copy link
Contributor

It looks good to me 👍 It appears that you've gone with @simurai's suggestions on the tooltips and status bar text, so that sounds like the best solution for now. We can always come back and tweak it if things go awry.

@winstliu winstliu merged commit 4a3661f into master Dec 10, 2016
@winstliu winstliu deleted the wl-super-dynamic-updates-tooltip branch December 10, 2016 20:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Updates status bar should indicate when a package is updating

4 participants