Bug 332341 - Handling of Focus and Selection should be made more consistent.
Summary: Handling of Focus and Selection should be made more consistent.
Status: NEW
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy GEF (MVC) (show other bugs)
Version: 3.6.1   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: gef-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-10 16:04 EST by Alexander Nyßen CLA
Modified: 2011-04-07 16:31 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Nyßen CLA 2010-12-10 16:04:35 EST
Currently, AbstractEditPartViewer and SelectionManager handle focus as follows:
1) select(EditPart) will not change focus if the edit part to be selected
already is the focus part; it will unset the focus part (and notify the former
focus part about loss of focus) otherwise.
2) appendSelection(EditPart) will not change focus if the appended edit part
already is the focus part; it will unset the focus part (and notify the former
focus part about loss of focus) otherwise.
3) deselect does not affect focus at all.
4) deselectAll unsets focus part, notifying the former focus part about loss of
focus.

While this is not really an inconsistency I think it may be hard to understand
for clients that a primary selected edit part may either explicitly (if it was
the focus part before being selected) as well as implicitly (if there is no
focus part) have focus, and that it may or may not get notified about gain/loss
of focus (if the viewer gains or looses focus) dependent on its former state.
Further, an explicit focus part may not only be set in case it differs from the
primary selection, but also in case it corresponds to it; while on the other
side, a primary selected edit part may also have (implicit) focus while not
being the focus part.

To improve understandability I think it would be better that an explicit focus
part is only set in case it differs from the primary selection (or in case
there is no selection and an edit part other than the contents or root should
have focus). This way, an edit part that has keyboard focus may either be the
explicit focus part (in case not being selected primary) or it may be the
implicit focus part (when being selected primary and no other edit part is set
as explicit focus part) and both states could be clearly separated. 

Concerning the notification of edit parts about gaining and loosing focus (via
EditPart#setFocus()), I think the two following options would be consistent:

a) every edit part that gains/looses focus should be notified about this (via
EditPart#setFocus()), independent of whether it is the explicit or implicit
(being the primary selection) focus part of the viewer.

b) only the explicit focus part should be notified about gain/loss of focus
(via EditPart#getFocus()). An implicit focus part (primary selected) should not
get notified.

While b) is the current behavior, which pretty much corresponds to what is
needed for how focus is displayed today (implicit focus part do only show their
primary selection state and no explicit focus border), I think a) would be the
more consistent option. This way, the fact that the edit part viewer gains or
looses focus could e.g. be graphically displayed by implicit (primary selected)
focus parts as well. Further, clients could rely on the fact that - in case an
edit part gains or looses focus - it always gets notified about this.
Comment 1 Randy Hudson CLA 2010-12-10 18:09:10 EST
You've missed a few details about the current (at least intended) behavior. The goals were to only set focus when the keyboard was being used instead of the mouse.  Rendering focus is distracting otherwise, for example, when CTRL+Clicking on something w/ the mouse to take it out of a multi-selection.

Even when the keyboard is used, I think setting focus is avoided when simply navigating around (so, changing the only selected part).

Editparts are notified of focus when they should visually display focus, not when they have the implied focus.
Comment 2 Alexander Nyßen CLA 2010-12-10 18:53:44 EST
(In reply to comment #1)
> You've missed a few details about the current (at least intended) behavior. The
> goals were to only set focus when the keyboard was being used instead of the
> mouse.  Rendering focus is distracting otherwise, for example, when
> CTRL+Clicking on something w/ the mouse to take it out of a multi-selection.
> 

Yes, I thought that this was the intended behavior (because setFocus is directly "forwarded" to the edit policies). However, the current behavior does not fully correspond to this policy. I.e. if you move the keyboard to an unselected edit part, then use CTRL + mouse click to append it to the current selection, the edit parts continues to show focus, while the mouse was used for interaction. Corresponding to the intended policy it think it would be consistent if the edit part looses focus in such a situation. That's what I subsume under option b). Here my point is to at least make the current behavior consistent to that policy.

> Even when the keyboard is used, I think setting focus is avoided when simply
> navigating around (so, changing the only selected part).
> 

Correct, focus is only moved on if CTRL is pressed when navigating.

> Editparts are notified of focus when they should visually display focus, not
> when they have the implied focus.

And that's what I address with option a). I think it would be more consistent, easier to understand for clients, and also more flexible to use setFocus() to actually notify edit parts about receiving/loosing focus rather than for just the graphical bits (I think that could be decided by the edit policies). This way, clients that always want to show the focus additional to the selection (independent of the selection) could also achieve this, while the current behavior w.r.t. to displaying focus feedback could be created by simply preventing edit policies to show focus feedback in case their host is also primary selected. Using setFocus() this way would also allow that a primary selected edit part could display gain/loss of focus in case the graphical viewer itself gains or losses focus.
Comment 3 Randy Hudson CLA 2010-12-13 10:03:41 EST
> (independent of the selection) could also achieve this, while the current
> behavior w.r.t. to displaying focus feedback could be created by simply
> preventing edit policies to show focus feedback in case their host is also
> primary selected.

If the user is moving focus around using CTRL+ARROW, focus should not "disappear" when it passes over the current primary selection.
Comment 4 Alexander Nyßen CLA 2010-12-13 10:24:49 EST
(In reply to comment #3)
> > (independent of the selection) could also achieve this, while the current
> > behavior w.r.t. to displaying focus feedback could be created by simply
> > preventing edit policies to show focus feedback in case their host is also
> > primary selected.
> 
> If the user is moving focus around using CTRL+ARROW, focus should not
> "disappear" when it passes over the current primary selection.

Well, with option a) such a decision could be left to the clients (i.e. to the edit policies), where - I agree - the default behavior should be to show focus in such a case.
Comment 5 Alexander Nyßen CLA 2011-04-07 16:31:10 EDT
Handling of focus and selection is also inconsistent within RulerViewer, where focus is also shown to indicate selection. I think we should come up with a consistent handling of focus and selection in all viewers.

Maybe it would be best (in difference to what I sketched in a) or b)) to handle them as two orthogonal concepts that can be handled independently. That implies that focus should always be explicitly assigned and indicated, independent of the selection state (in case a user uses the mouse to select, he of course also assigns the focus part). This would be the simplest and most straight-forward algorithm.