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

Two icons for representing notification progress are confusing #91469

Closed
isidorn opened this issue Feb 25, 2020 · 12 comments
Closed

Two icons for representing notification progress are confusing #91469

isidorn opened this issue Feb 25, 2020 · 12 comments
Assignees
Labels
polish Cleanup and polish issue ux User experience issues verification-needed Verification of issue is requested verified Verification succeeded workbench-notifications Notification widget issues
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Feb 25, 2020

Refs: #91163

We showing notificatino progress we render two icons in the status bar. The ball and the spinner.
I am not a big fan of this appraoch as they seem like two disconnected icons to me. We should somehow marry them, or show a progress in some other way.
On the very least they should share some backgroun color / outline

Screenshot 2020-02-25 at 21 01 59

@bpasero bpasero added polish Cleanup and polish issue ux User experience issues workbench-notifications Notification widget issues labels Feb 26, 2020
@bpasero bpasero added this to the February 2020 milestone Feb 26, 2020
@bpasero
Copy link
Member

bpasero commented Feb 26, 2020

Agree. @misolori might need your help. I am fine having another bell icon that just has a different decoration. Maybe we leave the animation out for laters.

@bpasero
Copy link
Member

bpasero commented Feb 26, 2020

For example:

image

@isidorn
Copy link
Contributor Author

isidorn commented Feb 26, 2020

Yeah that would be pretty cool. If the spinning is ike a spinning badge on the bell.

@bpasero
Copy link
Member

bpasero commented Feb 26, 2020

I doubt we can animate just that part of the bell. It would be static and not spinning.

Unless maybe with some tricks we can overlay 2.

@miguelsolorio
Copy link
Contributor

miguelsolorio commented Feb 27, 2020

You can now use bell-progress though it's a bit hard to make out the smaller icon (had to reduce the amount of lines to make it stand out more):

image

Yea, if we want to animate the second icon we'd need to use two icons and animate one.

@bpasero bpasero added the verification-needed Verification of issue is requested label Feb 27, 2020
@bpasero
Copy link
Member

bpasero commented Feb 27, 2020

I ended up not using this icon as I felt it was too subtle. What I did instead is to simply use the bell with a dot whenever a notification is in progress. I think this is easier to understand because this indicates that "something" is going on in the hidden notification center.

Verification: follow steps from #91163 and verify the bell shows a dot for as long as the notification is in progress.

@jrieken
Copy link
Member

jrieken commented Feb 27, 2020

Are we sure this isn't too subtle? We have never finishing save operations that can be behind this...

@bpasero
Copy link
Member

bpasero commented Feb 27, 2020

@jrieken the alternative is what I suggested earlier to promote a running save operation to the status bar where we usually show progress when not in notification.

@bpasero bpasero reopened this Feb 27, 2020
@bpasero
Copy link
Member

bpasero commented Feb 27, 2020

Went back to the original model where we would indicate the progress in the status bar whenever the notification is hidden:

Kapture 2020-02-27 at 12 55 36

Verification:

  • craft a action that you can use to bring up a notification with progress (see below)
  • as long as the notification is visible either as toast or in the notification center no status entry shows
  • once you hide the notification toast with ESC key, status bar shows progress for as long as the progress goes on
  • once you hide the notification center, status bar shows progress for as long as the progress goes on
  • ensure the status bar entry can be clicked to open the notification center
  • ensure the status bar entry is always showing the latest progress step message
  • ensure the status bar entry goes away when the progress is done
  • bring up multiple notifications with progress and see that things still work ok (note: the status bar currently only supports to show the last progress message that was set, not mutliple)
class ProgressAction extends Action {

	static readonly ID = 'workbench.action.ProgressAction';
	static readonly LABEL = nls.localize('ProgressAction', "ProgressAction");

	static counter = 0;

	constructor(
		id: string,
		label: string,
		@IProgressService private readonly progressService: IProgressService
	) {
		super(id, label);

		ProgressAction.counter++;
	}

	run(): Promise<void> {
		this.progressService.withProgress({
			location: ProgressLocation.Notification,
			title: `[${ProgressAction.counter}]Some long running operation`,
			cancellable: true
		}, progress => {
			progress.report({ increment: 0, total: 100 });

			let counter = 0;
			setInterval(() => {
				progress.report({ increment: 10, message: 'Increment from [microsoft.com](http://microsoft.com) ' + (++counter), total: 100 });
			}, 1000);

			return timeout(10000);
		});

		return Promise.resolve();
	}
}

@joaomoreno
Copy link
Member

@bpasero Fantastic verification steps!

@bpasero
Copy link
Member

bpasero commented Feb 27, 2020

Thanks for verifying so fast 👍

@miguelsolorio
Copy link
Contributor

Since we won't be using the subtle icon (which I agree with) I'll remove it from our library

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
polish Cleanup and polish issue ux User experience issues verification-needed Verification of issue is requested verified Verification succeeded workbench-notifications Notification widget issues
Projects
None yet
Development

No branches or pull requests

5 participants