From ca5107f59990ff4ec3d333475d32d87a37f7dc41 Mon Sep 17 00:00:00 2001 From: JFH <20402845+jfhenon@users.noreply.github.com> Date: Sun, 13 Mar 2022 19:37:27 +0100 Subject: [PATCH] Fix #736 (#738) * fix issue #736 * Update ext-storage.js * 7.1.3 --- CHANGES.md | 2 + package-lock.json | 4 +- package.json | 2 +- .../extensions/ext-storage/ext-storage.js | 1 + src/editor/panels/LayersPanel.js | 15 +-- src/svgcanvas/draw.js | 99 +++++++++++-------- src/svgcanvas/svgcanvas.js | 1 + 7 files changed, 67 insertions(+), 57 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1da53d04..b5e4375c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,4 +1,6 @@ # SVG-Edit CHANGES +## 7.1.3 +- fix issue #736. could not move layers ## 7.1.2 - add the current document title in the toolbar - allow user extensions to define optional parameters diff --git a/package-lock.json b/package-lock.json index bb8afee6..3c6ed008 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "svgedit", - "version": "7.1.2", + "version": "7.1.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "svgedit", - "version": "7.1.2", + "version": "7.1.3", "license": "(MIT AND Apache-2.0 AND ISC AND LGPL-3.0-or-later AND X11)", "dependencies": { "@babel/polyfill": "7.12.1", diff --git a/package.json b/package.json index 15519b6f..5efb2856 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "svgedit", - "version": "7.1.2", + "version": "7.1.3", "description": "Powerful SVG-Editor for your browser ", "main": "dist/Editor.js", "module": "dist/Editor.js", diff --git a/src/editor/extensions/ext-storage/ext-storage.js b/src/editor/extensions/ext-storage/ext-storage.js index c7e73dc4..672bfea4 100644 --- a/src/editor/extensions/ext-storage/ext-storage.js +++ b/src/editor/extensions/ext-storage/ext-storage.js @@ -98,6 +98,7 @@ export default { svgEditor.loadFromString(cached) const name = storage.getItem(`title-${key}`) ?? 'untitled.svg' svgEditor.topPanel.updateTitle(name) + svgEditor.layersPanel.populateLayers() } } diff --git a/src/editor/panels/LayersPanel.js b/src/editor/panels/LayersPanel.js index 0d7ca386..6247c82b 100644 --- a/src/editor/panels/LayersPanel.js +++ b/src/editor/panels/LayersPanel.js @@ -153,11 +153,7 @@ class LayersPanel { index (el) { if (!el) return -1 - let i = 0 - do { - i++ - } while (el === el.previousElementSibling) - return i + return Array.from(document.querySelector('#layerlist tbody').children).indexOf(el) } /** @@ -181,12 +177,9 @@ class LayersPanel { * @returns {void} */ moveLayer (pos) { - const total = this.editor.svgCanvas.getCurrentDrawing().getNumLayers() - - let curIndex = (this.index(document.querySelector('#layerlist tr.layersel')) - 1) - if (curIndex > 0 || curIndex < total - 1) { - curIndex += pos - this.editor.svgCanvas.setCurrentLayerPosition(total - curIndex - 1) + const curPos = this.editor.svgCanvas.indexCurrentLayer() + if (curPos !== -1) { + this.editor.svgCanvas.setCurrentLayerPosition(curPos - pos) this.populateLayers() } } diff --git a/src/svgcanvas/draw.js b/src/svgcanvas/draw.js index 0faf6d77..222b22d7 100644 --- a/src/svgcanvas/draw.js +++ b/src/svgcanvas/draw.js @@ -342,38 +342,31 @@ export class Drawing { return null } - let oldpos - for (oldpos = 0; oldpos < layerCount; ++oldpos) { - if (this.all_layers[oldpos] === this.current_layer) { break } - } - // some unknown error condition (current_layer not in all_layers) - if (oldpos === layerCount) { return null } + const oldpos = this.indexCurrentLayer() + if ((oldpos === -1) || (oldpos === newpos)) { return null } - if (oldpos !== newpos) { - // if our new position is below us, we need to insert before the node after newpos - const currentGroup = this.current_layer.getGroup() - const oldNextSibling = currentGroup.nextSibling + // if our new position is below us, we need to insert before the node after newpos + const currentGroup = this.current_layer.getGroup() + const oldNextSibling = currentGroup.nextSibling - let refGroup = null - if (newpos > oldpos) { - if (newpos < layerCount - 1) { - refGroup = this.all_layers[newpos + 1].getGroup() - } + let refGroup = null + if (newpos > oldpos) { + if (newpos < layerCount - 1) { + refGroup = this.all_layers[newpos + 1].getGroup() + } // if our new position is above us, we need to insert before the node at newpos - } else { - refGroup = this.all_layers[newpos].getGroup() - } - this.svgElem_.insertBefore(currentGroup, refGroup) // Ok to replace with `refGroup.before(currentGroup);`? - - this.identifyLayers() - this.setCurrentLayer(this.getLayerName(newpos)) - - return { - currentGroup, - oldNextSibling - } + } else { + refGroup = this.all_layers[newpos].getGroup() + } + this.svgElem_.insertBefore(currentGroup, refGroup) // Ok to replace with `refGroup.before(currentGroup);`? + + this.identifyLayers() + this.setCurrentLayer(this.getLayerName(newpos)) + + return { + currentGroup, + oldNextSibling } - return null } /** @@ -405,7 +398,7 @@ export class Drawing { // Remove current layer's group this.current_layer.removeGroup() // Remove the current layer and set the previous layer as the new current layer - const index = this.all_layers.indexOf(this.current_layer) + const index = this.indexCurrentLayer() if (index > 0) { const name = this.current_layer.getName() this.current_layer = this.all_layers[index - 1] @@ -451,6 +444,17 @@ export class Drawing { return false } + /** + * Sets the current layer. If the name is not a valid layer name, then this + * function returns `false`. Otherwise it returns `true`. This is not an + * undo-able action. + * @param {string} name - The name of the layer you want to switch to. + * @returns {boolean} `true` if the current layer was switched, otherwise `false` + */ + indexCurrentLayer () { + return this.all_layers.indexOf(this.current_layer) + } + /** * Deletes the current layer from the drawing and then clears the selection. * This function then calls the 'changed' handler. This is an undoable action. @@ -580,7 +584,7 @@ export class Drawing { } // Update layer containers and current_layer. - const index = this.all_layers.indexOf(this.current_layer) + const index = this.indexCurrentLayer() if (index >= 0) { this.all_layers.splice(index + 1, 0, layer) } else { @@ -763,11 +767,20 @@ export const init = (canvas) => { * @function module:draw.identifyLayers * @returns {void} */ -export const identifyLayers = function () { +export const identifyLayers = () => { leaveContext() svgCanvas.getCurrentDrawing().identifyLayers() } +/** +* get current index +* @function module:draw.identifyLayers +* @returns {void} +*/ +export const indexCurrentLayer = () => { + return svgCanvas.getCurrentDrawing().indexCurrentLayer() +} + /** * Creates a new top-level layer in the drawing with the given name, sets the current layer * to it, and then clears the selection. This function then calls the 'changed' handler. @@ -778,7 +791,7 @@ export const identifyLayers = function () { * @fires module:svgcanvas.SvgCanvas#event:changed * @returns {void} */ -export const createLayer = function (name, hrService) { +export const createLayer = (name, hrService) => { const newLayer = svgCanvas.getCurrentDrawing().createLayer( name, historyRecordingService(hrService) @@ -797,7 +810,7 @@ export const createLayer = function (name, hrService) { * @fires module:svgcanvas.SvgCanvas#event:changed * @returns {void} */ -export const cloneLayer = function (name, hrService) { +export const cloneLayer = (name, hrService) => { // Clone the current layer and make the cloned layer the new current layer const newLayer = svgCanvas.getCurrentDrawing().cloneLayer(name, historyRecordingService(hrService)) @@ -813,7 +826,7 @@ export const cloneLayer = function (name, hrService) { * @fires module:svgcanvas.SvgCanvas#event:changed * @returns {boolean} `true` if an old layer group was found to delete */ -export const deleteCurrentLayer = function () { +export const deleteCurrentLayer = () => { const { BatchCommand, RemoveElementCommand } = svgCanvas.history let currentLayer = svgCanvas.getCurrentDrawing().getCurrentLayer() const { nextSibling } = currentLayer @@ -838,7 +851,7 @@ export const deleteCurrentLayer = function () { * @param {string} name - The name of the layer you want to switch to. * @returns {boolean} true if the current layer was switched, otherwise false */ -export const setCurrentLayer = function (name) { +export const setCurrentLayer = (name) => { const result = svgCanvas.getCurrentDrawing().setCurrentLayer(toXml(name)) if (result) { svgCanvas.clearSelection() @@ -855,7 +868,7 @@ export const setCurrentLayer = function (name) { * @fires module:svgcanvas.SvgCanvas#event:changed * @returns {boolean} Whether the rename succeeded */ -export const renameCurrentLayer = function (newName) { +export const renameCurrentLayer = (newName) => { const drawing = svgCanvas.getCurrentDrawing() const layer = drawing.getCurrentLayer() if (layer) { @@ -877,7 +890,7 @@ export const renameCurrentLayer = function (newName) { * 0 and (number of layers - 1) * @returns {boolean} `true` if the current layer position was changed, `false` otherwise. */ -export const setCurrentLayerPosition = function (newPos) { +export const setCurrentLayerPosition = (newPos) => { const { MoveElementCommand } = svgCanvas.history const drawing = svgCanvas.getCurrentDrawing() const result = drawing.setCurrentLayerPosition(newPos) @@ -896,7 +909,7 @@ export const setCurrentLayerPosition = function (newPos) { * @param {boolean} bVisible - Whether the layer should be visible * @returns {boolean} true if the layer's visibility was set, false otherwise */ -export const setLayerVisibility = function (layerName, bVisible) { +export const setLayerVisibility = (layerName, bVisible) => { const { ChangeElementCommand } = svgCanvas.history const drawing = svgCanvas.getCurrentDrawing() const prevVisibility = drawing.getLayerVisibility(layerName) @@ -923,7 +936,7 @@ export const setLayerVisibility = function (layerName, bVisible) { * @param {string} layerName - The name of the layer you want to which you want to move the selected elements * @returns {boolean} Whether the selected elements were moved to the layer. */ -export const moveSelectedToLayer = function (layerName) { +export const moveSelectedToLayer = (layerName) => { const { BatchCommand, MoveElementCommand } = svgCanvas.history // find the layer const drawing = svgCanvas.getCurrentDrawing() @@ -955,7 +968,7 @@ export const moveSelectedToLayer = function (layerName) { * @param {module:history.HistoryRecordingService} hrService * @returns {void} */ -export const mergeLayer = function (hrService) { +export const mergeLayer = (hrService) => { svgCanvas.getCurrentDrawing().mergeLayer(historyRecordingService(hrService)) svgCanvas.clearSelection() leaveContext() @@ -967,7 +980,7 @@ export const mergeLayer = function (hrService) { * @param {module:history.HistoryRecordingService} hrService * @returns {void} */ -export const mergeAllLayers = function (hrService) { +export const mergeAllLayers = (hrService) => { svgCanvas.getCurrentDrawing().mergeAllLayers(historyRecordingService(hrService)) svgCanvas.clearSelection() leaveContext() @@ -981,7 +994,7 @@ export const mergeAllLayers = function (hrService) { * @fires module:svgcanvas.SvgCanvas#event:contextset * @returns {void} */ -export const leaveContext = function () { +export const leaveContext = () => { const len = disabledElems.length const dataStorage = svgCanvas.getDataStorage() if (len) { @@ -1009,7 +1022,7 @@ export const leaveContext = function () { * @fires module:svgcanvas.SvgCanvas#event:contextset * @returns {void} */ -export const setContext = function (elem) { +export const setContext = (elem) => { const dataStorage = svgCanvas.getDataStorage() leaveContext() if (typeof elem === 'string') { diff --git a/src/svgcanvas/svgcanvas.js b/src/svgcanvas/svgcanvas.js index 0e17c2de..c1ecbeae 100644 --- a/src/svgcanvas/svgcanvas.js +++ b/src/svgcanvas/svgcanvas.js @@ -1303,6 +1303,7 @@ class SvgCanvas { this.setCurrentLayer = draw.setCurrentLayer this.renameCurrentLayer = draw.renameCurrentLayer this.setCurrentLayerPosition = draw.setCurrentLayerPosition + this.indexCurrentLayer = draw.indexCurrentLayer this.setLayerVisibility = draw.setLayerVisibility this.moveSelectedToLayer = draw.moveSelectedToLayer this.mergeLayer = draw.mergeLayer