Bug 2229 - Usability: Progress Dialog on Save is distracting (1GEAR0D)
Summary: Usability: Progress Dialog on Save is distracting (1GEAR0D)
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 2.0   Edit
Hardware: All All
: P2 major (vote)
Target Milestone: 2.0 F3   Edit
Assignee: Adam Schlegel CLA
QA Contact:
URL:
Whiteboard:
Keywords: usability
: 18452 (view as bug list)
Depends on:
Blocks:
 
Reported: 2001-10-10 22:30 EDT by Erich Gamma CLA
Modified: 2002-06-10 11:55 EDT (History)
3 users (show)

See Also:


Attachments
Implementation Issues (7.12 KB, text/html)
2002-05-27 16:38 EDT, Adam Schlegel CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Erich Gamma CLA 2001-10-10 22:30:36 EDT
EG (5/25/01 9:29:44 PM)
	usability feedback from GDA
Progress dialog on saving is very distracting/annoying 
it also causes the entire workbench to flash because the progress 
dialog causes the workbench to lose/gain focus. important

NOTES:
Comment 1 Genady Beryozkin CLA 2001-11-17 07:54:16 EST
Sometimes the save operation takes a long time, and I want to
watch the progress. 
It should be displayed only if the predicted save time is longer than a second 
or two.
Comment 2 Erich Gamma CLA 2001-11-19 03:07:59 EST
A time out based progress dialog would be a good solution.

An alternative would be to show the progress in status bar so that it is less
distracting.
Comment 3 Alex Balut CLA 2002-03-29 17:59:28 EST
You could measure the time taken by the save operation and store it.. At the next save operation, if the stored time is > 2s you show the progress dialogue.. and store the new time.
Comment 4 Nick Edgar CLA 2002-05-03 09:46:55 EDT
EG says:

This one is important to us, since it is on the critical path for incrementally 
compiling a method. 
Not showing this dialog would significantly improve the perceived performance 
of small incremental builds.
Comment 5 Nick Edgar CLA 2002-05-26 15:10:18 EDT
We've encountered some stumbling blocks in being able to do this.
Things are complicated by the fact that Save is done in the UI thread.
Adam to provide more details.
Reducing priority since this is not a stop-ship.
Comment 6 Erich Gamma CLA 2002-05-26 15:54:34 EDT
Then could save use at least the progress bar in the status line provided by 
ApplicationWindow? This would also reduce the flicker. 

BTW, this progress bar should use an Eclipse style stop icon (the one that is 
currently shown was manually crafted during the LeapFrog age).
Comment 7 Erich Gamma CLA 2002-05-26 16:22:43 EDT
If the problem is that Save is run in the UI thread than another solution is to 
check the elapsed time on each progress tick and to only show the progress 
dialog after some elapsed time. To handle the case of a long no progress tick 
interval an SWT timer could be used.
Comment 8 Nick Edgar CLA 2002-05-27 10:12:04 EDT
We're looking further at ApplicationWindow.run.
It has the following problems:
- it disables the window (everything but the shell itself), so it will lose 
input
- it's flashy
- the disabling does not currently cover the shortcut bar, since this is not 
known to ApplicationWindow

However, 
  - ProgressMonitorDialog blocks input anyway, so this is not a major loss
  - ProgressMonitorDialog is more flashy
  - we can fix the shortcut bar problem

Also, note that we cannot use an SWT timer if we're running in the UI thread, 
since it requires the event loop to be running.
Comment 9 Adam Schlegel CLA 2002-05-27 13:40:50 EDT
ApplicationWindow.run uses get/setEnabled methods on the various components of 
the window to disable them during the operation. However, these methods lose 
the current keyboard focus. There does not appear to be the necessary API to 
save/restore the focus state.

If the user is making changes in the current editor, and then saves via Ctrl+s, 
it is inappropriate to take keyboard focus away from the curent editor. This 
amounts to an even worse usability problem than presently exists.
Comment 10 Adam Schlegel CLA 2002-05-27 16:38:05 EDT
I have compiled a more detailed summary of the problems encountered in trying 
to fix this problem. 
Comment 11 Adam Schlegel CLA 2002-05-27 16:38:58 EDT
Created attachment 1078 [details]
Implementation Issues
Comment 12 Nick Edgar CLA 2002-05-29 15:06:53 EDT
I have released Adam's patch to use the status line progress monitor for save.
I also changed the icon to a better one we had lying around (will double-check 
with Linda).

Erich, could you please try it out.  You will need to load our changes from 
HEAD.

There are some known layout problems which we want to fix up.
The progress bar and cancel button appear on the right, after the Java editors' 
status line contributions for Writable, Insert, and cursor position.
They should appear to the left of them, taking up the right side of the message 
area without moving the other contributions.
Comment 13 Erich Gamma CLA 2002-05-30 06:14:26 EDT
Tried it - incremental builds feel now much faster. This is great progress!

Issues:
- other top-level shells are not disabled only the application window. If a 
build takes longer you can start a second build from another workbench window 
and you will deadlock. In LeapFrog we had code that disabled all Shells to 
handle this case. Dirk wrote the code and he is trying to find it again. This 
approach has worked well in LeapFrog.

- since incremental builds are fast the appearance looks like flicker. Showing 
the progress after some elapsed time (2 seconds) would help. The check-the-
elapsed-time-on-a-progress-tick should work. To handle the case of no tick for 
an extended period you could use a timer thread.

If the disabling problem is fixed then we would like to use the progress in the 
status line for other actions as well (e.g. opening a type hierarchy, opening 
the all types dialog). Is there any reason to not doing so?
Comment 14 Erich Gamma CLA 2002-05-30 06:35:52 EDT
Dirk found the code in a VA/Java repository

	protected Object aboutToStart(boolean enableCancelButton) {
		Control focusControl= null;
		if (Utilities.okToUse(fShell)) {
			// Save the focus control
			focusControl= fShell.getDisplay().getFocusControl();
			if (focusControl != null && focusControl.getShell() != 
fShell)
				focusControl= null;
				
			// Set the busy cursor to all shells.
			Display d= fShell.getDisplay();
			fWaitCursor= new Cursor(d, SWT.CURSOR_WAIT);
			CursorUtil.setCursor(d, fWaitCursor);
			
			// Deactivate shell
			setEnableShells(fShell.getDisplay(), fShell, false);
			setActive(false, enableCancelButton);	
		}
		return focusControl;
	}

	private void setEnableShells(Display display, Shell exception, boolean 
enable) {
		Shell[] shells= display.getShells();
		for (int i= 0; i < shells.length; i++) {
			Shell shell= shells[i];
			if (shell != exception) {
				shell.setEnabled(enable);
			}
		}
	}

	/**
	 * Helper method to activate or deactivate a ApplicationWindow.
	 */
	private void setActive(boolean b, boolean cancelButton) {
		if (Utilities.okToUse(fShell)) {
			fShell.getMenuBar().setEnabled(b);
			if (fToolBarManager != null)
				fToolBarManager.getControl(fShell).setEnabled
(b);
			Control w= getContents();
			if (w != null)
				w.setEnabled(b);
			setOperationCancelButtonEnabled(cancelButton);	
		}		
	}

	/**
	 * Sets the given cursor to all shells currently active
	 * for the given display.
	 */
	public static void setCursor(Display d, Cursor c) {
		if (d != null) {
			Shell[] shells= d.getShells();
			for (int i= 0; i < shells.length; i++)
				shells[i].setCursor(c);
		}
	}
Comment 15 Nick Edgar CLA 2002-05-31 09:42:55 EDT
Adam is already looking at disabling other windows.
We will also see what we can do about flicker.

As for using it in other places, I'm concerned about different operations using 
different progress monitors.  I think it would be confusing if the user could 
not predict where a given operation would report progress.
If it's just save, it's predictable.
But, if you think using it in other places makes sense for JDT, go ahead.
Comment 16 Nick Edgar CLA 2002-05-31 09:43:20 EDT
Escalating to P1 for the multi window deadlock fix.
Comment 17 Erich Gamma CLA 2002-05-31 10:37:09 EDT
our primary candidate is opening a type hierarchy, we currently do not show 
progress just a busy cursor. It is very similar to save, the operation is fast 
most of the time but can be very slow when (60 seconds) when opening a type 
hierarchy on object.
Comment 18 Nick Edgar CLA 2002-05-31 11:33:42 EDT
*** Bug 18452 has been marked as a duplicate of this bug. ***
Comment 19 Adam Schlegel CLA 2002-05-31 16:35:41 EDT
There is a fix for the multi-window deadlock problem in the repository.

I am still working on the flicker. There already is a 1/2 second delay 
associated with showing the progress monitor in the status bar. The delay can 
be made longer by default, but there will always be ther case where we get 
flicker when an operation takes just a little longer than the delay.

If this code is going to get wider use, it might be worth making it so that the 
progress monitor tries to guess if it is going to flicker based on the amount 
of progress that has happened so far.
Comment 20 Nick Edgar CLA 2002-06-05 16:02:32 EDT
Need to fix the layout problem.  The progress bar and cancel should not cause 
other contributions to move -- they should take space from the message label.
Comment 21 Nick Edgar CLA 2002-06-10 11:55:13 EDT
Fixed in 20020610.
This (re)introduced bug 19786 which is being pursued separately.