Cycle list is different for middle-click and Alt+click

[joshdoe]

Middle-clicking cycles through all nearby nodes and ways, but Alt+click only cycles through objects of the type already selected. The former is implemented in MapStatus.java​, and uses getAllNearest() from NavigatableComponent.java​. The latter is implemented in SelectAction.java​ and uses getNearestNodes() if a node is already selected, or getNearestWays() if a way is already selected.

This inconsistency doesn't make sense to me. I made the trivial modification so Alt+click uses getAllNearest(), and it seems to work as I'd expect.

Any particular reason it was implemented this way?

[joshdoe]

use all nearest objects when Alt+clicking

[joshdoe]

I'm pretty sure because this patch is so trivial I must be missing something important. Maybe cmuelle8 can shed some light since he worked on the cycling code?

[skyper]

Replying to joshdoe:

I'm pretty sure because this patch is so trivial I must be missing something important. Maybe cmuelle8 can shed some light since he worked on the cycling code?

Why do we need two different ways for the same action ?

Please, be careful with ALT + Mouse as it is already used by many window-manager in Linux !

[joshdoe]

Replying to skyper:

Replying to joshdoe:

I'm pretty sure because this patch is so trivial I must be missing something important. Maybe cmuelle8 can shed some light since he worked on the cycling code?

Why do we need two different ways for the same action ?

Middle-mouse button is not always available or easy to use (smaller laptops, netbooks), and honestly the pop-up just gets in the way most of the time.

Please, be careful with ALT + Mouse as it is already used by many window-manager in Linux !

This behavior has already been implemented for over a year (see #5457) and there are alternatives as I just happened to mention here.

[cmuelle8]

Replying to joshdoe:

This behavior has already been implemented for over a year (see #5457) and there are alternatives as I just happened to mention here.

thx for the clarity in this comment - it's a fairly good summary on primitive cycling..

I've never looked at the code that builds the list when middle click is used, but it should not be so difficult to use same code paths for list building, I guess. imho those lists do not have to be identical from a user perspective. however, if that simplifies the code it wouldn't hurt either.

greetings

[akks]

By the way, why did not anyone apply such useful patch? Are there any introduced problems?

[stoecker]

Replying to akks:

By the way, why did not anyone apply such useful patch? Are there any introduced problems?

Nobody had time for a review. Apply it, when you reviewed it.

[akks]

It is all ok with this patch, it works as desired.

The only question before commiting - why does not we add relations to getAllNearest() (to the end of list)?
Users then can have fast and easy accesible tool to select relations. Can it lead to some ergonomic or performance problems? :)

[joshdoe]

Replying to akks:

It is all ok with this patch, it works as desired.

The only question before commiting - why does not we add relations to getAllNearest() (to the end of list)?
Users then can have fast and easy accesible tool to select relations. Can it lead to some ergonomic or performance problems? :)

See #7314 which does exactly what you mentiom. I've been using these two patches for the past month and have found them very useful. Thanks for taking the time to look at them.

[akks]

Oh, I had some feeling of Deja-Vu writing last comment :) Sorry. These shortcut issues are bad for my memory)))

I will apply both patches then.

[akks]

In 5016/josm:

Patch by JoshDoe [Faster relation selection with Alt+click and middle-click], fix #7314, #7317

[cmuelle8]

The patch in #7314 is fine, I think. From a user perpective #7317 sucks though - it significantly changes user handling.

I did some real live mapping with 5016/josm and it is cumbersome to click through all the objects [rels/ways/nodes] as opposed to the list that is most probable.

E.g. if I want to build a nodes selection, I usually select a node, then use modifier Strg and click on the next node, not necessarily being in the same cycle group, of course. Since 5016/josm this is impossible, I have to cycle through ALL objects near the node starting from the second node I select.

In essence this means, I cannot select e.g. 5 random nodes anymore by simply clicking on them, for each node but the first I have to cycle through ALL objects nearby and 'wait' until the right node shows up, then move on.

This introduced behavior seems dumb, since the editor can (and did) do better - if I click on or really close to a node, the probability is very high, that I indeed want to cycle a node list (if present at all, usually just one node if uncluttered) OR a way list.

The argument that is given to cycleSetup has an important information which is, if the user clicked 'more' on a node or more close to a way that is connected to a node. Depending on this, node or way lists have been cycled in the past. This was very usable, since those lists can grow very long in dense areas and doing a preselection of what list to be build based on the input to cycleSetup() automagically split this to do the right thing (tm).

While I love #7314, I cannot really live with #7317. My suggestion would be the following:

• build a getAllNearestRelations() method in NavigatableComponent.java separately
• revert the patch in SelectAction.java and append the output of getAllNearestRelations() to
  • either the node list
  • or the way list
• .. depending on which one is built in cycleSetup()

Appending only the relevant relations leads to the following possibilities for including relations in the cycle list:

• cycle nodes + all relations as given by the combined relation list
• cycle ways + all relations as given by the combined relation list
• cycle nodes + those nearby relations that have at least one of the nodes on the node list as member
• cycle ways + those nearby relations that have at least one of the way on the way list as member

Greetings,
Christian

[joshdoe]

Replying to cmuelle8:

While I love #7314, I cannot really live with #7317. My suggestion would be the following:

• build a getAllNearestRelations() method in NavigatableComponent.java separately
• revert the patch in SelectAction.java and append the output of getAllNearestRelations() to
  • either the node list
  • or the way list
• .. depending on which one is built in cycleSetup()

I haven't found this functionality inconvenient, but I can understand in certain use cases it could be frustrating. I think your suggestion is a fine one, though it still means you'll have to cycle through relations when you just want nodes. Perhaps this should be an advanced preference? There could be three levels, only nodes OR ways, (nodes OR ways) AND relations, and then all objects. Question is what the default should be. I'd tend to go with the middle one, which is your suggestion.