Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JOSM #22308 - Add option to toggle layer read-only status to popup menu #99

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions src/org/openstreetmap/josm/actions/ToggleEditLockLayerAction.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.actions;

import static org.openstreetmap.josm.tools.I18n.tr;

import java.awt.Component;
import java.awt.event.ActionEvent;
import java.util.List;

import javax.swing.AbstractAction;
import javax.swing.JCheckBoxMenuItem;

import org.openstreetmap.josm.data.osm.Lockable;
import org.openstreetmap.josm.gui.dialogs.LayerListDialog;
import org.openstreetmap.josm.gui.layer.Layer;
import org.openstreetmap.josm.gui.layer.Layer.LayerAction;
import org.openstreetmap.josm.tools.CheckParameterUtil;
import org.openstreetmap.josm.tools.ImageProvider;

/**
* An action enabling/disabling the {@linkplain Lockable#lock() read-only flag}
* of the layer specified in the constructor.
* @param <L> Type of layer the action should be instantiated for
*
* @since xxx
*/
public class ToggleEditLockLayerAction<L extends Layer & Lockable> extends AbstractAction implements LayerAction {

private final L layer;

/**
* Construct a new {@code ToggleEditLockLayerAction}
* @param layer the layer for which to toggle the {@linkplain Lockable#lock() read-only flag}
*
* @since xxx
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @since xxx

Not needed since this is an entirely new class -- the @since xxx on L25 covers everything in the file.

*/
public ToggleEditLockLayerAction(L layer) {
super(tr("Prevent modification"));
CheckParameterUtil.ensureParameterNotNull(layer, "layer");
putValue(SHORT_DESCRIPTION, tr("Prevent/allow changes being made in this layer"));
new ImageProvider("lock").getResource().attachImageIcon(this, true);
this.layer = layer;
}

@Override
public void actionPerformed(ActionEvent e) {
if (layer.isLocked()) {
layer.unlock();
} else {
layer.lock();
}

layer.invalidate();
LayerListDialog.getInstance().repaint();
}

@Override
public Component createMenuComponent() {
JCheckBoxMenuItem item = new JCheckBoxMenuItem(this);
item.setSelected(layer.isLocked());
return item;
}

@Override
public boolean supportLayers(List<Layer> layers) {
return layers.size() == 1 && layers.get(0) instanceof Lockable;
Copy link

@tsmock tsmock Aug 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason why you are requiring layer size to be 1? Why not

Suggested change
return layers.size() == 1 && layers.get(0) instanceof Lockable;
return layers.stream().allMatch(Lockable.class::isInstance);

EDIT: You'll want to implement MultiLayerAction if you want it to apply to multiple layers.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modeled this after the prevent upload toggle action which works exactly the same way. Not sure if locking/unlocking multiple layers at once is ever really needed.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class ToggleUploadDiscouragedLayerAction extends AbstractAction implement
*/
public ToggleUploadDiscouragedLayerAction(OsmDataLayer layer) {
super(tr("Discourage upload"));
putValue(SHORT_DESCRIPTION, tr("Allow/disallow upload of changes made in this layer"));
new ImageProvider("no_upload").getResource().attachImageIcon(this, true);
this.layer = layer;
setEnabled(layer.isUploadable());
Expand Down
2 changes: 2 additions & 0 deletions src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.openstreetmap.josm.actions.AutoScaleAction;
import org.openstreetmap.josm.actions.ExpertToggleAction;
import org.openstreetmap.josm.actions.RenameLayerAction;
import org.openstreetmap.josm.actions.ToggleEditLockLayerAction;
import org.openstreetmap.josm.actions.ToggleUploadDiscouragedLayerAction;
import org.openstreetmap.josm.data.APIDataSet;
import org.openstreetmap.josm.data.Bounds;
Expand Down Expand Up @@ -743,6 +744,7 @@ public Action[] getMenuEntries() {
new RenameLayerAction(getAssociatedFile(), this)));
if (ExpertToggleAction.isExpert()) {
actions.add(new ToggleUploadDiscouragedLayerAction(this));
actions.add(new ToggleEditLockLayerAction<>(this));
}
actions.addAll(Arrays.asList(
new ConsistencyTestAction(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.actions;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.openstreetmap.josm.data.osm.DataSet;
import org.openstreetmap.josm.gui.MainApplication;
import org.openstreetmap.josm.gui.layer.OsmDataLayer;
import org.openstreetmap.josm.testutils.JOSMTestRules;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

/**
* Unit tests for class {@link ToggleEditLockLayerAction}.
*/
final class ToggleEditLockLayerActionTest {

/**
* Setup test.
*/
@RegisterExtension
@SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
public static JOSMTestRules test = new JOSMTestRules().main().projection();

/**
* Test null parameter
*/
@Test
void testNullLayer() {
assertThrows(IllegalArgumentException.class, () -> new ToggleEditLockLayerAction<OsmDataLayer>(null));
}

/**
* Test edit lock toggle functionality
* @param locked Initial layer lock status
*/
@ParameterizedTest
@ValueSource(booleans = {true, false})
void testLayerLockToggle(boolean locked) {
OsmDataLayer testLayer = new OsmDataLayer(new DataSet(), "testLayerLock", null);
MainApplication.getLayerManager().addLayer(testLayer);
if (locked) {
testLayer.lock();
}
// Sanity check
assertEquals(locked, testLayer.isLocked());
ToggleEditLockLayerAction<OsmDataLayer> action = new ToggleEditLockLayerAction<>(testLayer);
action.actionPerformed(null);
assertNotEquals(locked, testLayer.isLocked());
action.actionPerformed(null);
assertEquals(locked, testLayer.isLocked());
}
}