diff --git a/packages/base/src/commands/BaseCommandIDs.ts b/packages/base/src/commands/BaseCommandIDs.ts index 6fbc82318..b958bebe2 100644 --- a/packages/base/src/commands/BaseCommandIDs.ts +++ b/packages/base/src/commands/BaseCommandIDs.ts @@ -29,10 +29,8 @@ export const newGeoTiffEntry = 'jupytergis:newGeoTiffEntry'; export const newGeoParquetEntry = 'jupytergis:newGeoParquetEntry'; // Layer and group actions -export const renameLayer = 'jupytergis:renameLayer'; -export const removeLayer = 'jupytergis:removeLayer'; -export const renameGroup = 'jupytergis:renameGroup'; -export const removeGroup = 'jupytergis:removeGroup'; +export const renameSelected = 'jupytergis:renameSelected'; +export const removeSelected = 'jupytergis:removeSelected'; export const moveLayersToGroup = 'jupytergis:moveLayersToGroup'; export const moveLayerToNewGroup = 'jupytergis:moveLayerToNewGroup'; diff --git a/packages/base/src/commands/index.ts b/packages/base/src/commands/index.ts index ad4690e72..e2407b935 100644 --- a/packages/base/src/commands/index.ts +++ b/packages/base/src/commands/index.ts @@ -7,7 +7,6 @@ import { IJupyterGISModel, JgisCoordinates, LayerType, - SelectionType, SourceType, } from '@jupytergis/schema'; import { JupyterFrontEnd } from '@jupyterlab/application'; @@ -534,41 +533,41 @@ export function addCommands( /** * LAYERS and LAYER GROUP actions. */ - commands.addCommand(CommandIDs.renameLayer, { - label: trans.__('Rename Layer'), - execute: async () => { + commands.addCommand(CommandIDs.renameSelected, { + label: trans.__('Rename'), + isEnabled: () => { const model = tracker.currentWidget?.model; - await Private.renameSelectedItem(model, 'layer'); + const selected = model?.localState?.selected?.value; + return !!selected && Object.keys(selected).length === 1; }, - }); - - commands.addCommand(CommandIDs.removeLayer, { - label: trans.__('Remove Layer'), - execute: () => { + execute: async () => { const model = tracker.currentWidget?.model; - Private.removeSelectedItems(model, 'layer', selection => { - model?.removeLayer(selection); - }); + const selected = model?.localState?.selected?.value; - commands.notifyCommandChanged(CommandIDs.toggleStoryPresentationMode); + if (!model || !selected) { + return; + } + + await Private.renameSelectedItem(model); }, }); - commands.addCommand(CommandIDs.renameGroup, { - label: trans.__('Rename Group'), - execute: async () => { + commands.addCommand(CommandIDs.removeSelected, { + label: trans.__('Remove'), + isEnabled: () => { const model = tracker.currentWidget?.model; - await Private.renameSelectedItem(model, 'group'); + const selected = model?.localState?.selected?.value; + return !!selected && Object.keys(selected).length > 0; }, - }); - - commands.addCommand(CommandIDs.removeGroup, { - label: trans.__('Remove Group'), execute: async () => { const model = tracker.currentWidget?.model; - Private.removeSelectedItems(model, 'group', selection => { - model?.removeLayerGroup(selection); - }); + const selected = model?.localState?.selected?.value; + + if (!model || !selected) { + return; + } + + await Private.removeSelectedItems(model); }, }); @@ -659,7 +658,7 @@ export function addCommands( label: trans.__('Rename Source'), execute: async () => { const model = tracker.currentWidget?.model; - await Private.renameSelectedItem(model, 'source'); + await Private.renameSelectedItem(model); }, }); @@ -667,16 +666,7 @@ export function addCommands( label: trans.__('Remove Source'), execute: () => { const model = tracker.currentWidget?.model; - Private.removeSelectedItems(model, 'source', selection => { - if (!(model?.getLayersBySource(selection).length ?? true)) { - model?.sharedModel.removeSource(selection); - } else { - showErrorMessage( - 'Remove source error', - 'The source is used by a layer.', - ); - } - }); + Private.removeSelectedSources(model); }, }); @@ -1230,52 +1220,71 @@ namespace Private { }; } - export function removeSelectedItems( - model: IJupyterGISModel | undefined, - itemTypeToRemove: SelectionType, - removeFunction: (id: string) => void, - ) { + export function removeSelectedItems(model: IJupyterGISModel | undefined) { const selected = model?.localState?.selected?.value; - if (!selected) { + if (!selected || !model) { console.error('Failed to remove selected item -- nothing selected'); return; } - for (const selection in selected) { - if (selected[selection].type === itemTypeToRemove) { - removeFunction(selection); + for (const id of Object.keys(selected)) { + const item = selected[id]; + + switch (item.type) { + case 'layer': + model.removeLayer(id); + break; + case 'group': + model.removeLayerGroup(id); + break; } } } export async function renameSelectedItem( model: IJupyterGISModel | undefined, - itemType: SelectionType, ) { - const selectedItems = model?.localState?.selected.value; + const selectedItems = model?.localState?.selected?.value; if (!selectedItems || !model) { - console.error(`No ${itemType} selected`); + console.error('No item selected'); return; } - let itemId = ''; + const ids = Object.keys(selectedItems); + if (ids.length === 0) { + return; + } - // If more then one item is selected, only rename the first - for (const id in selectedItems) { - if (selectedItems[id].type === itemType) { - itemId = id; - break; - } + const itemId = ids[0]; + const item = selectedItems[itemId]; + + if (!item.type) { + return; } - if (!itemId) { + model.setEditingItem(item.type, itemId); + } + + export function removeSelectedSources(model: IJupyterGISModel | undefined) { + const selected = model?.localState?.selected?.value; + + if (!selected || !model) { return; } - // Set editing state - component will show inline input - model.setEditingItem(itemType, itemId); + for (const id of Object.keys(selected)) { + if (model.getLayersBySource(id).length > 0) { + showErrorMessage( + 'Remove source error', + 'The source is used by a layer.', + ); + continue; + } + + model.sharedModel.removeSource(id); + } } export function executeConsole(tracker: JupyterGISTracker): void { diff --git a/packages/base/src/keybindings.json b/packages/base/src/keybindings.json index d0f7cdf3f..4ea3eafba 100644 --- a/packages/base/src/keybindings.json +++ b/packages/base/src/keybindings.json @@ -30,24 +30,14 @@ "selector": ".data-jgis-keybinding .jp-gis-source" }, { - "command": "jupytergis:removeLayer", + "command": "jupytergis:removeSelected", "keys": ["Delete"], - "selector": ".data-jgis-keybinding .jp-gis-layerItem" - }, - { - "command": "jupytergis:renameLayer", - "keys": ["F2"], - "selector": ".jp-gis-layerItem" - }, - { - "command": "jupytergis:removeGroup", - "keys": ["Delete"], - "selector": ".data-jgis-keybinding .jp-gis-layerGroupHeader" + "selector": ".data-jgis-keybinding" }, { - "command": "jupytergis:renameGroup", + "command": "jupytergis:renameSelected", "keys": ["F2"], - "selector": ".jp-gis-layerGroupHeader" + "selector": ".data-jgis-keybinding" }, { "command": "jupytergis:executeConsole", diff --git a/python/jupytergis_lab/src/index.ts b/python/jupytergis_lab/src/index.ts index 3b83782f8..82c7db63c 100644 --- a/python/jupytergis_lab/src/index.ts +++ b/python/jupytergis_lab/src/index.ts @@ -58,6 +58,8 @@ const plugin: JupyterFrontEndPlugin = { ); }; + const LAYER = '.jp-gis-layerItem:not(.jp-gis-layerGroup)'; + createDefaultLayerRegistry(layerBrowserRegistry); const stateDbManager = GlobalStateDbManager.getInstance(); stateDbManager.initialize(state); @@ -87,32 +89,32 @@ const plugin: JupyterFrontEndPlugin = { // LAYERS and LAYER GROUPS context menu app.contextMenu.addItem({ command: CommandIDs.symbology, - selector: '.jp-gis-layerItem', + selector: LAYER, rank: 1, }); // Separator app.contextMenu.addItem({ type: 'separator', - selector: '.jp-gis-layerPanel', + selector: LAYER, rank: 1, }); app.contextMenu.addItem({ - command: CommandIDs.removeLayer, - selector: '.jp-gis-layerItem', + command: CommandIDs.removeSelected, + selector: LAYER, rank: 2, }); app.contextMenu.addItem({ - command: CommandIDs.renameLayer, - selector: '.jp-gis-layerItem', + command: CommandIDs.renameSelected, + selector: LAYER, rank: 2, }); app.contextMenu.addItem({ command: CommandIDs.zoomToLayer, - selector: '.jp-gis-layerItem', + selector: LAYER, rank: 2, }); @@ -128,7 +130,7 @@ const plugin: JupyterFrontEndPlugin = { // Add the Download submenu to the context menu app.contextMenu.addItem({ type: 'submenu', - selector: '.jp-gis-layerItem', + selector: LAYER, rank: 2, submenu: downloadSubmenu, }); @@ -148,7 +150,7 @@ const plugin: JupyterFrontEndPlugin = { app.contextMenu.addItem({ type: 'submenu', - selector: '.jp-gis-layerItem', + selector: LAYER, rank: 2, submenu: processingSubmenu, }); @@ -161,7 +163,7 @@ const plugin: JupyterFrontEndPlugin = { app.contextMenu.addItem({ type: 'submenu', - selector: '.jp-gis-layerItem', + selector: LAYER, rank: 2, submenu: moveLayerSubmenu, }); @@ -171,13 +173,13 @@ const plugin: JupyterFrontEndPlugin = { ); app.contextMenu.addItem({ - command: CommandIDs.removeGroup, + command: CommandIDs.removeSelected, selector: '.jp-gis-layerGroupHeader', rank: 2, }); app.contextMenu.addItem({ - command: CommandIDs.renameGroup, + command: CommandIDs.renameSelected, selector: '.jp-gis-layerGroupHeader', rank: 2, }); diff --git a/ui-tests/tests/contextmenu.spec.ts b/ui-tests/tests/contextmenu.spec.ts index bf2c2b1c5..e4dfcdcca 100644 --- a/ui-tests/tests/contextmenu.spec.ts +++ b/ui-tests/tests/contextmenu.spec.ts @@ -24,14 +24,14 @@ test.describe('context menu', () => { .getByText('Open Topo Map') .click({ button: 'right' }); - const text = page.getByRole('menu').getByText('Remove Layer'); + const text = page.getByRole('menu').getByText('Remove'); await expect(text).toBeVisible(); }); test('right click on group should open group menu', async ({ page }) => { await page.getByText('level 1 group').click({ button: 'right' }); - const text = page.getByRole('menu').getByText('Remove Group'); + const text = page.getByRole('menu').getByText('Remove'); await expect(text).toBeVisible(); }); @@ -74,7 +74,7 @@ test.describe('context menu', () => { await expect(page.getByText('new group', { exact: true })).toHaveCount(1); }); - test('clicking remove layer should remove the layer from the tree', async ({ + test('clicking remove should remove the layer from the tree', async ({ page, }) => { // Create new layer first @@ -102,12 +102,12 @@ test.describe('context menu', () => { button: 'right', }); - await page.getByRole('menu').getByText('Remove Layer').click(); + await page.getByRole('menu').getByText('Remove').click(); expect(layerInTree).not.toBeVisible(); }); - test('clicking remove group should remove the group from the tree', async ({ + test('clicking remove should remove the group from the tree', async ({ page, }) => { const firstItem = page @@ -119,7 +119,7 @@ test.describe('context menu', () => { .getByText('level 1 group') .click({ button: 'right' }); - await page.getByRole('menu').getByText('Remove Group').click(); + await page.getByRole('menu').getByText('Remove').click(); await expect(firstItem).not.toBeVisible(); });