Index: /trunk/src/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerAction.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerAction.java (revision 18836) +++ /trunk/src/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerAction.java (revision 18837) @@ -6,5 +6,7 @@ import java.awt.Component; import java.awt.event.ActionEvent; +import java.util.HashMap; import java.util.List; +import java.util.Map; import javax.swing.AbstractAction; @@ -46,4 +48,6 @@ if (!SaveLayersDialog.saveUnsavedModifications(selectedLayers, SaveLayersDialog.Reason.DELETE)) return; + final Map layerMap = model.selectedIndices().filter(i -> model.getLayer(i) != null) + .collect(HashMap::new, (map, value) -> map.put(value, model.getLayer(value)), HashMap::putAll); for (Layer l: selectedLayers) { if (model.getLayerManager().containsLayer(l)) { @@ -51,4 +55,29 @@ // this is why we need to check if every layer is still in the list of selected layers. model.getLayerManager().removeLayer(l); + } + } + // Set the next active layer to the next visible layer + if (layerMap.size() == 1) { + final int selected = Math.min(layerMap.keySet().iterator().next(), model.getRowCount() - 1); + int currentLayerIndex = selected; + Layer layer = model.getLayer(currentLayerIndex); + // If the user has the last layer selected, we need to wrap around. + boolean reversed = false; + while (layer != null && !layer.isVisible() && currentLayerIndex < model.getRowCount() && currentLayerIndex >= 0) { + if (reversed) { + currentLayerIndex--; + } else { + currentLayerIndex++; + } + if (currentLayerIndex == model.getRowCount()) { + reversed = true; + currentLayerIndex = selected; + } + layer = model.getLayer(currentLayerIndex); + } + if (layer != null) { + model.getLayerManager().setActiveLayer(layer); + // Reset the selection + model.getSelectionModel().setSelectionInterval(selected, selected); } } Index: /trunk/test/unit/org/openstreetmap/josm/actions/DeleteLayerActionTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/actions/DeleteLayerActionTest.java (revision 18836) +++ /trunk/test/unit/org/openstreetmap/josm/actions/DeleteLayerActionTest.java (revision 18837) @@ -5,25 +5,17 @@ import static org.junit.jupiter.api.Assertions.assertNull; -import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.Test; 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; +import org.openstreetmap.josm.testutils.annotations.Main; +import org.openstreetmap.josm.testutils.annotations.Projection; /** * Unit tests for class {@link DeleteLayerAction}. */ +@Main +@Projection final class DeleteLayerActionTest { - - /** - * Setup test. - */ - @RegisterExtension - @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") - public static JOSMTestRules test = new JOSMTestRules().main().projection(); - /** * Unit test of {@link DeleteLayerAction#actionPerformed} Index: /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerActionTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerActionTest.java (revision 18837) +++ /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/layer/DeleteLayerActionTest.java (revision 18837) @@ -0,0 +1,128 @@ +package org.openstreetmap.josm.gui.dialogs.layer; + +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Objects; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.dialogs.LayerListDialog; +import org.openstreetmap.josm.gui.layer.Layer; +import org.openstreetmap.josm.gui.layer.OsmDataLayer; +import org.openstreetmap.josm.testutils.annotations.Main; +import org.openstreetmap.josm.testutils.annotations.Projection; + +/** + * Test class for {@link DeleteLayerAction} + */ +@Main +@Projection +class DeleteLayerActionTest { + private LayerListDialog.LayerListModel model; + private DeleteLayerAction deleteLayerAction; + private ShowHideLayerAction showHideLayerAction; + + @BeforeEach + void setup() { + final AtomicInteger counter = new AtomicInteger(); + final Supplier layerSupplier = () -> new OsmDataLayer(new DataSet(), "testActiveLayer" + counter.getAndIncrement(), null); + for (int i = 0; i < 10; i++) { + MainApplication.getLayerManager().addLayer(layerSupplier.get()); + } + final LayerListDialog layerListDialog = LayerListDialog.getInstance(); + this.model = layerListDialog.getModel(); + this.deleteLayerAction = layerListDialog.createDeleteLayerAction(); + this.showHideLayerAction = layerListDialog.createShowHideLayerAction(); + } + + @Test + void testSetActiveLayerOnlyOneVisible0to8() { + hideRange(0, 8); + assertEquals(9, model.getSelectedLayers().size()); + assertEquals(1, model.getLayers().stream().filter(Layer::isVisible).count()); + deleteLayerAction.actionPerformed(null); + assertEquals(1, model.getSelectedLayers().size()); + final Layer layer = assertInstanceOf(OsmDataLayer.class, model.getLayer(0)); + assertNotNull(layer); + assertTrue(layer.isVisible()); + assertSame(layer, model.getLayerManager().getActiveLayer()); + assertEquals("testActiveLayer0", layer.getName()); + } + + @Test + void testSetActiveLayerOnlyOneVisible1to9() { + hideRange(1, 9); + assertEquals(9, model.getSelectedLayers().size()); + assertEquals(1, model.getLayers().stream().filter(Layer::isVisible).count()); + deleteLayerAction.actionPerformed(null); + assertEquals(1, model.getSelectedLayers().size()); + final Layer layer = assertInstanceOf(OsmDataLayer.class, model.getLayer(0)); + assertNotNull(layer); + assertTrue(layer.isVisible()); + assertSame(layer, model.getLayerManager().getActiveLayer()); + assertEquals("testActiveLayer9", layer.getName()); + } + + @Test + void testRemoveMiddleActiveWithSurroundingHiddenLayers() { + hideRange(3, 3); + hideRange(5, 5); + final Layer toRemove = model.getLayer(4); + assertNotNull(toRemove); + assertTrue(toRemove.isVisible()); + assertFalse(Objects.requireNonNull(model.getLayer(3)).isVisible()); + assertFalse(Objects.requireNonNull(model.getLayer(5)).isVisible()); + model.getLayerManager().setActiveLayer(toRemove); + model.setSelectedLayer(toRemove); + deleteLayerAction.actionPerformed(null); + assertSame(model.getLayerManager().getActiveLayer(), model.getLayer(5)); + assertEquals("testActiveLayer3", Objects.requireNonNull(model.getLayer(5)).getName()); + assertAll(model.getLayers().stream().map(layer -> () -> assertNotSame(toRemove, layer))); + } + + @Test + void testRemoveTopActiveWithSurroundingHiddenLayers() { + hideRange(1, 1); + final Layer toRemove = model.getLayer(0); + assertNotNull(toRemove); + assertTrue(toRemove.isVisible()); + assertFalse(Objects.requireNonNull(model.getLayer(1)).isVisible()); + model.getLayerManager().setActiveLayer(toRemove); + model.setSelectedLayer(toRemove); + deleteLayerAction.actionPerformed(null); + assertSame(model.getLayerManager().getActiveLayer(), model.getLayer(1)); + assertEquals("testActiveLayer7", Objects.requireNonNull(model.getLayer(1)).getName()); + assertAll(model.getLayers().stream().map(layer -> () -> assertNotSame(toRemove, layer))); + } + + @Test + void testRemoveBottomActiveWithSurroundingHiddenLayers() { + hideRange(8, 8); + final Layer toRemove = model.getLayer(9); + assertNotNull(toRemove); + assertTrue(toRemove.isVisible()); + assertFalse(Objects.requireNonNull(model.getLayer(8)).isVisible()); + model.getLayerManager().setActiveLayer(toRemove); + model.setSelectedLayer(toRemove); + deleteLayerAction.actionPerformed(null); + assertSame(model.getLayerManager().getActiveLayer(), model.getLayer(7)); + assertEquals("testActiveLayer2", Objects.requireNonNull(model.getLayer(7)).getName()); + assertAll(model.getLayers().stream().map(layer -> () -> assertNotSame(toRemove, layer))); + } + + private void hideRange(int start, int end) { + model.getSelectionModel().setSelectionInterval(start, end); + showHideLayerAction.actionPerformed(null); + } +}