Fix #736 (#738)

* fix issue #736

* Update ext-storage.js

* 7.1.3

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

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",

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",

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()
       }
     }
 

src/editor/panels/LayersPanel.js

@@ -153,11 +153,7 @@ class LayersPanel {
 
   index (el) {
     if (!el) return -1
-    let i = 0
+    return Array.from(document.querySelector('#layerlist tbody').children).indexOf(el)
-    do {
-      i++
-    } while (el === el.previousElementSibling)
-    return i
   }
 
   /**
@@ -181,12 +177,9 @@ class LayersPanel {
    * @returns {void}
    */
   moveLayer (pos) {
-    const total = this.editor.svgCanvas.getCurrentDrawing().getNumLayers()
+    const curPos = this.editor.svgCanvas.indexCurrentLayer()
-
+    if (curPos !== -1) {
-    let curIndex = (this.index(document.querySelector('#layerlist tr.layersel')) - 1)
+      this.editor.svgCanvas.setCurrentLayerPosition(curPos - pos)
-    if (curIndex > 0 || curIndex < total - 1) {
-      curIndex += pos
-      this.editor.svgCanvas.setCurrentLayerPosition(total - curIndex - 1)
       this.populateLayers()
     }
   }

src/svgcanvas/draw.js

@@ -342,38 +342,31 @@ export class Drawing {
       return null
     }
 
-    let oldpos
+    const oldpos = this.indexCurrentLayer()
-    for (oldpos = 0; oldpos < layerCount; ++oldpos) {
+    if ((oldpos === -1) || (oldpos === newpos)) { return null }
-      if (this.all_layers[oldpos] === this.current_layer) { break }
-    }
-    // some unknown error condition (current_layer not in all_layers)
-    if (oldpos === layerCount) { return null }
 
-    if (oldpos !== newpos) {
+    // if our new position is below us, we need to insert before the node after newpos
-      // if our new position is below us, we need to insert before the node after newpos
+    const currentGroup = this.current_layer.getGroup()
-      const currentGroup = this.current_layer.getGroup()
+    const oldNextSibling = currentGroup.nextSibling
-      const oldNextSibling = currentGroup.nextSibling
 
-      let refGroup = null
+    let refGroup = null
-      if (newpos > oldpos) {
+    if (newpos > oldpos) {
-        if (newpos < layerCount - 1) {
+      if (newpos < layerCount - 1) {
-          refGroup = this.all_layers[newpos + 1].getGroup()
+        refGroup = this.all_layers[newpos + 1].getGroup()
-        }
+      }
       // if our new position is above us, we need to insert before the node at newpos
-      } else {
+    } else {
-        refGroup = this.all_layers[newpos].getGroup()
+      refGroup = this.all_layers[newpos].getGroup()
-      }
+    }
-      this.svgElem_.insertBefore(currentGroup, refGroup) // Ok to replace with `refGroup.before(currentGroup);`?
+    this.svgElem_.insertBefore(currentGroup, refGroup) // Ok to replace with `refGroup.before(currentGroup);`?
-
+
-      this.identifyLayers()
+    this.identifyLayers()
-      this.setCurrentLayer(this.getLayerName(newpos))
+    this.setCurrentLayer(this.getLayerName(newpos))
-
+
-      return {
+    return {
-        currentGroup,
+      currentGroup,
-        oldNextSibling
+      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') {

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