Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#18638 closed enhancement (fixed)

[PATCH] There should be a method to cycle through layers

Reported by: taylor.smock Owned by: team
Priority: normal Milestone: 20.02
Component: Core Version:
Keywords: layer, cycle Cc:

Description (last modified by taylor.smock)

I originally used CTRL+[/CTRL+] for the layer switches, but I figured SHIFT+{/SHIFT+} would be better ([ -> { due to SHIFT converting [ to {).

I haven't written tests yet -- I'm doing that now.

I probably didn't pick the right class to instantiate the new class in (I'm currently using LayerListDialog, but I figured that was probably the best place to put it).

Attachments (8)

18638.patch (6.0 KB ) - added by taylor.smock 4 years ago.
Initial patch (no tests)
18638.1.patch (10.5 KB ) - added by taylor.smock 4 years ago.
Switch back to [ and ] from { and } along with tests
18638.review.patch (10.9 KB ) - added by Don-vip 4 years ago.
codestyle fixed. Beware to please the Shortcut parser
18638_junit.jpg (25.5 KB ) - added by Don-vip 4 years ago.
18638.2.patch (10.0 KB ) - added by taylor.smock 4 years ago.
Split cycle up/cycle down actions into separate files, fix shortcut issues (single line), and some other pmd issues
18638.3.patch (2.1 KB ) - added by taylor.smock 4 years ago.
Actually initialize the actions (in LayerListDialog)
18638.shortcuts.patch (1.8 KB ) - added by GerdP 4 years ago.
18638.shortcuts_names.patch (2.8 KB ) - added by taylor.smock 4 years ago.
Modify GerdP's patch to add consistency in action UI descriptions

Download all attachments as: .zip

Change History (28)

by taylor.smock, 4 years ago

Attachment: 18638.patch added

Initial patch (no tests)

comment:1 by taylor.smock, 4 years ago

Description: modified (diff)

by taylor.smock, 4 years ago

Attachment: 18638.1.patch added

Switch back to [ and ] from { and } along with tests

comment:2 by taylor.smock, 4 years ago

Description: modified (diff)

comment:3 by taylor.smock, 4 years ago

Ping

comment:4 by Don-vip, 4 years ago

Milestone: 20.02

comment:5 by Don-vip, 4 years ago

Milestone: 20.0220.03

by Don-vip, 4 years ago

Attachment: 18638.review.patch added

codestyle fixed. Beware to please the Shortcut parser

comment:6 by Don-vip, 4 years ago

I've modified your patch to respect checkstyle rules and make sure the Shortcut parser will work properly. But I got three unit test failures on 6:

Last edited 4 years ago by Don-vip (previous) (diff)

by Don-vip, 4 years ago

Attachment: 18638_junit.jpg added

by taylor.smock, 4 years ago

Attachment: 18638.2.patch added

Split cycle up/cycle down actions into separate files, fix shortcut issues (single line), and some other pmd issues

in reply to:  6 comment:7 by taylor.smock, 4 years ago

I put it in the MapWithAI plugin temporarily, and it also complained about the tests. I fixed them, but forgot to update the patch here.
It turns out that (for whatever reason), the CI environment wasn't passing the modifier keys, IIRC. I ended up splitting the actions up, instead of trying to have one class (it worked on Mac OS X, but not in the CI environment).

I forgot to bring the changes back over to this bug report. I should have, so you wouldn't have duplicated some of the work.

comment:8 by Don-vip, 4 years ago

Resolution: fixed
Status: newclosed

In 15923/josm:

fix #18638 - add a method to cycle through layers (patch by taylor.smock, modified)

comment:9 by skyper, 4 years ago

Milestone 20.2

comment:10 by taylor.smock, 4 years ago

Resolution: fixed
Status: closedreopened

I forgot to add the initialization code to the patch.

Last edited 4 years ago by taylor.smock (previous) (diff)

by taylor.smock, 4 years ago

Attachment: 18638.3.patch added

Actually initialize the actions (in LayerListDialog)

comment:11 by Don-vip, 4 years ago

Resolution: fixed
Status: reopenedclosed

In 15929/josm:

fix #18638 - Initialize the cycle layer actions in LayerListDialog (patch by taylor.smock)

comment:12 by Klumbumbus, 4 years ago

Milestone: 20.0320.02

comment:13 by Zverikk, 4 years ago

By the way, [ and ] keys are used by the "todo" plugin to cycle through the todo list.

comment:14 by taylor.smock, 4 years ago

I knew that, which is why I used shift + [/shift + ] (which translates the ] to } and [ to {, which made testing interesting).

comment:15 by Zverikk, 4 years ago

Ah, indeed, thank you.

comment:16 by Hb---, 4 years ago

The names for the keyboard shortcuts and for the action seem not consistent to each other. Currently we have

Cycle layers up for the shortcut
Cycle layer up for the action and
Cycle up through layers for the hint.

Cycle layers down for the shortcut
Cycle layers for the down action and
Cycle through layers for the hint

Are different names for the shortcut and for the action necessary?

Why is plural used when only one layer is affected? This good action is not as shuffling the background layers around each other.

Suggestion:
Cycle layer up for shortcut and action
Cycle through data layers upwards for the hint.

Cycle layer down for the shortcut and action
Cycle through data layers downwards for the hint

comment:17 by GerdP, 4 years ago

FYI: I see this message in some unit tests, e.g. JoinAreasActionTest

2020-03-06 09:40:10.255 INFORMATION: Registered toolbar action cycle-layer overwritten: org.openstreetmap.josm.gui.dialogs.layer.CycleLayerUpAction gets org.openstreetmap.josm.gui.dialogs.layer.CycleLayerDownAction

by GerdP, 4 years ago

Attachment: 18638.shortcuts.patch added

comment:18 by GerdP, 4 years ago

Resolution: fixed
Status: closedreopened

My patch fixes only the issue reported in comment:17

by taylor.smock, 4 years ago

Attachment: 18638.shortcuts_names.patch added

Modify GerdP's patch to add consistency in action UI descriptions

comment:19 by Don-vip, 4 years ago

Resolution: fixed
Status: reopenedclosed

In 16090/josm:

fix #18638 - add consistency in action UI descriptions (patch by taylor.smock)

comment:20 by Don-vip, 4 years ago

In 16097/josm:

see #18638 - checkstyle

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.