Bug 180193 - [Dialogs] [open type] Confusing ellipsis in Open Type dialog
Summary: [Dialogs] [open type] Confusing ellipsis in Open Type dialog
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.3 M7   Edit
Assignee: Kevin McGuire CLA
QA Contact:
URL:
Whiteboard:
Keywords: polish
Depends on: 185453
Blocks:
  Show dependency tree
 
Reported: 2007-03-30 09:40 EDT by Jerome Lanneluc CLA
Modified: 2007-06-06 14:52 EDT (History)
6 users (show)

See Also:
Tod_Creasey: review+
Tod_Creasey: review? (susan)


Attachments
Screenshot of Open Type dialog (26.90 KB, image/jpeg)
2007-03-30 09:41 EDT, Jerome Lanneluc CLA
no flags Details
Changes concatenated messages to <task>: <subtask> (1.65 KB, patch)
2007-04-30 05:23 EDT, Markus Keller CLA
no flags Details | Diff
Patch to cover just the Open Dialog concat issue (918 bytes, patch)
2007-05-03 17:45 EDT, Kevin McGuire CLA
no flags Details | Diff
Improved patch (1.81 KB, patch)
2007-05-07 09:49 EDT, Markus Keller CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jerome Lanneluc CLA 2007-03-30 09:40:59 EDT
I20070327

As shown in the attached screenshot, the ellipsis is confusing. Does it mean that the message is truncated ? This would be weird since there is plenty of space. If it is to show that the user should expect something, shouldn't the ellipsis be at the end of the messae ?
Comment 1 Jerome Lanneluc CLA 2007-03-30 09:41:50 EDT
Created attachment 62489 [details]
Screenshot of Open Type dialog
Comment 2 Mike Wilson CLA 2007-04-05 11:05:55 EDT
(Kevin using Mcq's machine)
Adding myself to CC because this interests me from a consistency point of view.  I bet we do this in a number of places.
As a general guidelines, I'd say that we should use ellipses to show truncation only, but that's just an initial gut response.
Comment 3 Markus Keller CLA 2007-04-29 19:24:07 EDT
Indeed, this is a common pattern that occurs when a progress monitor task ends in an ellipsis ("Searching...") and has a subtask ("* files to index").

It's common usage to just concatenate the two messages with a space, see e.g. ProgressMonitorPart.taskLabel().

I don't think we should try to change this in the Open Type dialog only. If this is to be changed, then it should be a platform decision, and recommendations should be documented in IProgressMonitor.
Comment 4 Markus Keller CLA 2007-04-30 05:23:28 EDT
Created attachment 65371 [details]
Changes concatenated messages to <task>: <subtask>

The best way to fix this would be to use ": " as separator between the task name and the subtask name (instead of " "). Clients such as the Open Type dialog could then remove the ellipsis, and the concatenated message is still readable. Note that we can't add the colon to the "Searching" task name in the client, since we don't know when a subtask is present and when not.

The attached patch applies this change in the two places I found in the platform where messages are currently concatenated with " ".
Comment 5 Kevin McGuire CLA 2007-04-30 13:36:08 EDT
(In reply to comment #4)
> The best way to fix this would be to use ": " as separator between the task
> name and the subtask name (instead of " "). 

I like this suggestion and it would work better than "..."
Comment 6 Markus Keller CLA 2007-04-30 14:15:32 EDT
Interesting: The Progress view already uses ": ", see ProgressMessages.JobInfo_DoneNoProgressMessage.
Comment 7 Kevin McGuire CLA 2007-05-02 14:33:38 EDT
I think we should do this but its a bit late in the cycle for changing how task/subtask are concatenated, plus the knockon changes to places like Open Type.

Lets look at this start of 3.4.
Comment 8 Martin Aeschlimann CLA 2007-05-03 04:05:03 EDT
This is one of the items on the polish list, so we would like to tackle these.
It's just about fixing the case for the filtered item dialog. You're right that we should go through all other cases in 3.4.
Comment 9 Markus Keller CLA 2007-05-03 10:28:24 EDT
The fix in org/eclipse/jface/messages.properties is not good, since the code using it in ProgressMonitorPart is broken (shows colon even with empty subtask). I've added a better patch for that instance to bug 185347.

To fix this bug, only the small change in org/eclipse/ui/internal/messages.properties would be necessary.
Comment 10 Kevin McGuire CLA 2007-05-03 11:14:53 EDT
OK, I'm fine with fixing it for just FilteredItemsSelectionDialog_subtaskProgressMessage in org.eclipse.ui.

There are a number of actual dialogs affected, because the string is used in GranualMessageDialog which is used by org.eclipse.ui.dialogs.FilteredItemsSelectionDialog  which has the following subclasses:
  org.eclipse.jdt.internal.debug.ui.breakpoints.AddExceptionDialog
  org.eclipse.jdt.internal.debug.ui.launcher.DebugTypeSelectionDialog
  org.eclipse.ui.dialogs.FilteredResourcesSelectionDialog
  org.eclipse.jdt.internal.ui.packageview.GotoResourceAction.GotoResourceDialog
  org.eclipse.ui.views.navigator.GotoResourceDialog
  org.eclipse.ui.internal.ide.dialogs.OpenResourceDialog
  org.eclipse.jdt.internal.ui.dialogs.FilteredTypesSelectionDialog
  org.eclipse.jdt.internal.ui.dialogs.OpenTypeSelectionDialog
  org.eclipse.jdt.internal.ui.wizards.SuperInterfaceSelectionDialog

But, since they're all doing the same thing (showing list of items to select from), I can't think of how this change would be bad in some way for these.
Comment 11 Kevin McGuire CLA 2007-05-03 17:45:19 EDT
Created attachment 65860 [details]
Patch to cover just the Open Dialog concat issue
Comment 12 Kevin McGuire CLA 2007-05-04 11:10:20 EDT
Note I've logged bug #185453 to track the JDT portion of this fix.  It needs to be applied first otherwise the resulting string concatenation will look odd.
Comment 13 Markus Keller CLA 2007-05-07 09:49:45 EDT
Created attachment 66102 [details]
Improved patch

Also removes '...' from task names in FilteredItemsSelectionDialog.
Comment 14 Markus Keller CLA 2007-05-07 09:54:02 EDT
> Note I've logged bug #185453 to track the JDT portion of this fix.  It needs to
> be applied first otherwise the resulting string concatenation will look odd.

Sequence does not really matter -- both look odd:
With Platform/UI patch only:     Searching...: 9 files to index (20%)
With JDT/UI patch only:          Searching  9 files to index (20%)

JDT/UI is ready to release for tomorrow's I-build.
Comment 15 Kevin McGuire CLA 2007-05-07 15:22:35 EDT
Did a quick test with both patches, looks ok.  Tod will need to do the commit.
Comment 16 Markus Keller CLA 2007-05-09 08:53:26 EDT
I've released the JDT/UI patch (bug 185453) to HEAD.
Tod, could you please release the "Improved patch"?
Comment 17 Tod Creasey CLA 2007-05-09 10:28:02 EDT
+1 from me but we need a second committer. Susan could you review this please?
Comment 18 Markus Keller CLA 2007-05-09 11:23:34 EDT
> +1 from me but we need a second committer. Susan could you review this please?

Wow, does Platform/UI use an even stricter rule of engagement than the default RC1 rules? I am a committer, and AFAIK, Kevin is also a committer, so that would already be three pairs of eyes (in contrast to the required two)...
Comment 19 Kevin McGuire CLA 2007-05-09 12:55:48 EDT
(In reply to comment #18)
> Wow, does Platform/UI use an even stricter rule of engagement than the default
> RC1 rules? I am a committer, and AFAIK, Kevin is also a committer, so that
> would already be three pairs of eyes (in contrast to the required two)...

I am a committer but not on UI.  Guess it depends on whether you count the 2 by team or not (on SWT its same for me and we only had one other set of eyes).

Comment 20 Markus Keller CLA 2007-05-09 13:02:31 EDT
Never mind, the fix has already been released to HEAD ;-).
Comment 21 Susan McCourt CLA 2007-05-09 13:49:28 EDT
The fix is in HEAD, but the CVS comment references bug #184688.  Was the commit intentional or was it accidentally committed when changes for the other bug were committed?

At any rate, the changes look ok to me.
Comment 22 Kevin McGuire CLA 2007-05-10 14:57:25 EDT
Marking fixed since patches applied
Comment 23 Kevin McGuire CLA 2007-05-10 14:57:58 EDT
Verified in N0510 (although man those strings go by fast!)
Comment 24 Kevin McGuire CLA 2007-06-06 14:52:36 EDT
Removed "contributed" keyword as I assume it was tagged in error (don't see any external contributions)