Bug 521225 - Arrow key navigation skips list container elements
Summary: Arrow key navigation skips list container elements
Status: NEW
Alias: None
Product: Sirius
Classification: Modeling
Component: Diagram (show other bugs)
Version: 5.0.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: triaged
Depends on:
Blocks:
 
Reported: 2017-08-22 04:23 EDT by Dominik Mosen CLA
Modified: 2019-01-29 07:27 EST (History)
4 users (show)

See Also:


Attachments
short demonstration (170.42 KB, image/gif)
2017-08-22 04:23 EDT, Dominik Mosen CLA
no flags Details
samples to reproduce with Ecore (6.43 KB, application/x-zip-compressed)
2017-08-22 09:02 EDT, Julien Dupont CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dominik Mosen CLA 2017-08-22 04:23:37 EDT
Created attachment 269929 [details]
short demonstration

When navigating in a left or right aligned list container using the arrow keys, some elements can be skipped. It seems to have something to do with the length of the list element's label. Watch the attached gif for a short demonstration.

Steps to reproduce:
* Import the "Basic Family Modeler Definition - Advanced Tutorial Solution" from the Sirius Examples
* Set "Label Alignment" to left or right for the mapping named "ChildrenNode"
* Import the "Basic Family Sample Model" from the Sirius Examples
* Change "Fiona" to something with at least 10 characters
* Create a representation to Elias to display his relations
* Navigate through the children using the up and down arrow keys
Comment 1 Julien Dupont CLA 2017-08-22 09:01:57 EDT
Adding a new example based on ecore.
With the example (in bug.521225.design/samples) with naming E2PlusD works fine with E2PlusDe does not work. Does not work from 8 caracters.
Works fine with label centered but not with label Label Alignment to Left.
Comment 2 Julien Dupont CLA 2017-08-22 09:02:35 EDT
Created attachment 269934 [details]
samples to reproduce with Ecore
Comment 3 Tamas Miklossy CLA 2017-08-22 10:15:44 EDT
When it is expected to be fixed?
Comment 4 Pierre-Charles David CLA 2017-08-23 04:22:38 EDT
(In reply to Tamas Miklossy from comment #3)
> When it is expected to be fixed?

Hi. It's not on the roadmap for any release for now. That said this bug can move forward if you provide a fix for it or you contact Obeo for sponsored work.

I've pushed the analysis a little bit, and this is actually side effect of the GEF key navigation algorithm (see org.eclipse.gef.ui.parts.GraphicalViewerKeyHandler.findSibling(List, Point, int, EditPart)): in your example when "Dave" is selected and you hit the Down arrow key, the algorithm tries to locate the next sibling (among "1234567890", "Clara", and "Bryan") which is down from "Dave". However to compare the relative position of two siblings it uses their figure's center (see org.eclipse.gef.ui.parts.GraphicalViewerKeyHandler.getNavigationPoint(IFigure)), and in your case the center of "1234567890" is not considered "down" from "Dave", but "right", i.e. "1234567890" is long enough that its center point is more to the right of "Dave"'s center than it is down, and thus it is skipped).

This is consistent with what Julien mentioned that it "does not work from 8 characters", but it's not actually related to the asbolute size/number of characters, but to the relative sizes and positions of the figures (and the navigation direction).
Comment 5 Tamas Miklossy CLA 2017-08-23 06:48:01 EDT
I see. Thank you for your quick reply and your detailed analysis! What do you think it is possible to fix it in Sirius project or rather in GEF project?
Comment 6 Eclipse Genie CLA 2017-08-23 09:24:58 EDT
New Gerrit change created: https://git.eclipse.org/r/103532
Comment 7 Pierre-Charles David CLA 2017-08-23 09:25:55 EDT
(In reply to Tamas Miklossy from comment #5)
> I see. Thank you for your quick reply and your detailed analysis! What do
> you think it is possible to fix it in Sirius project or rather in GEF
> project?

Unfortunately, the methods we would need to override on the Sirius side (e.g. org.eclipse.gef.ui.parts.GraphicalViewerKeyHandler.navigateNextSibling(KeyEvent, int)) are package-visible, and thus can not be overridden. GEF 3 is considered legacy, and not maintained anymore, so I don't think we can change that. The only solution I see would involve copying navigateNextSibling() and all its requirements on the Sirius side in a custom GraphicalViewerKeyHandler, and then change the code there.

The draft at https://git.eclipse.org/r/103532 seems to fix the issue, but it's rather ugly with lots of copy/pasted code just to add a special case...
Comment 8 Tamas Miklossy CLA 2019-01-29 07:27:46 EST
When is it planned to merge the provided Gerrit change into the Sirius Code base? We would need this fix as soon as possible. Thanks!