Fixes for loading images into image nodes (#2964)

* Add warnings and errors for image loading

* Fixes and commenting for image loading

* Fix typo

* Small fixes due to review

lib/network/Images.js

@@ -19,8 +19,12 @@ class Images {
      * @return {Image} imageToLoadBrokenUrlOn   The image object
      */    
     _tryloadBrokenUrl (url, brokenUrl, imageToLoadBrokenUrlOn) {
-        //If any of the parameters aren't specified then exit the function because nothing constructive can be done
-        if (url === undefined || brokenUrl === undefined || imageToLoadBrokenUrlOn === undefined)  return;
+        //If these parameters aren't specified then exit the function because nothing constructive can be done
+        if (url === undefined || imageToLoadBrokenUrlOn === undefined)  return;
+        if (brokenUrl === undefined) {
+          console.warn("No broken url image defined");
+          return;
+        }
     
         //Clear the old subscription to the error event and put a new in place that only handle errors in loading the brokenImageUrl
         imageToLoadBrokenUrlOn.onerror = () => {

lib/network/modules/components/Node.js

@@ -141,21 +141,7 @@ class Node {
 
     this.choosify(options);
 
-    // load the images
-    if (this.options.image !== undefined) {
-      if (this.imagelist) {
-        if (typeof this.options.image === 'string') {
-          this.imageObj = this.imagelist.load(this.options.image, this.options.brokenImage, this.id);
-        } else {
-          this.imageObj = this.imagelist.load(this.options.image.unselected, this.options.brokenImage, this.id);
-          this.imageObjAlt = this.imagelist.load(this.options.image.selected, this.options.brokenImage, this.id);
-        }
-      }
-      else {
-        throw "No imagelist provided";
-      }
-    }
-
+    this._load_images();
     this.updateLabelModule(options);
     this.updateShape(currentShape);
     this.labelModule.propagateFonts(this.nodeOptions, options, this.defaultOptions);
@@ -167,6 +153,46 @@ class Node {
   }
 
 
+  /**
+   * Load the images from the options, for the nodes that need them.
+   *
+   * TODO: The imageObj members should be moved to CircularImageBase.
+   *       It's the only place where they are required.
+   *
+   * @private
+   */
+  _load_images() {
+    // Don't bother loading for nodes without images
+    if (this.options.shape !== 'circularImage' && this.options.shape !== 'image') {
+      return;
+    }
+
+    if (this.options.image === undefined) {
+      throw "Option image must be defined for node type '" + this.options.shape + "'";
+    }
+
+    if (this.imagelist === undefined) {
+      throw "Internal Error: No images provided";
+    }
+
+    if (typeof this.options.image === 'string') {
+      this.imageObj = this.imagelist.load(this.options.image, this.options.brokenImage, this.id);
+    } else {
+      if (this.options.image.unselected === undefined) {
+        throw "No unselected image provided";
+      }
+
+      this.imageObj = this.imagelist.load(this.options.image.unselected, this.options.brokenImage, this.id);
+
+      if (this.options.image.selected !== undefined) {
+        this.imageObjAlt = this.imagelist.load(this.options.image.selected, this.options.brokenImage, this.id);
+      } else {
+        this.imageObjAlt = undefined;
+      }
+    }
+  }
+
+
   /**
    * This process all possible shorthands in the new options and makes sure that the parentOptions are fully defined.
    * Static so it can also be used by the handler.

lib/network/modules/components/nodes/shapes/CircularImage.js

@@ -29,14 +29,8 @@ class CircularImage extends CircleImageBase {
   }
 
   draw(ctx, x, y, selected, hover, values) {
-    // switch images depending on 'selected' if imageObjAlt exists
-    if (this.imageObjAlt) {
-      this.switchImages(selected);
-    }
-
-    this.selected = selected;
-
-    this.resize(ctx, selected, hover);
+    this.switchImages(selected);
+    this.resize();
     this.left = x - this.width / 2;
     this.top = y - this.height / 2;
 

lib/network/modules/components/nodes/shapes/Image.js

@@ -16,14 +16,8 @@ class Image extends CircleImageBase {
   }
 
   draw(ctx, x, y, selected, hover, values) {
-    // switch images depending on 'selected' if imageObjAlt exists
-    if (this.imageObjAlt) {
-      this.switchImages(selected);
-    }
-
-    this.selected = selected;
-
-    this.resize(ctx, selected, hover);
+    this.switchImages(selected);
+    this.resize();
     this.left = x - this.width / 2;
     this.top = y - this.height / 2;
 

lib/network/modules/components/nodes/util/CircleImageBase.js

@@ -24,9 +24,25 @@ class CircleImageBase extends NodeBase {
 
   setOptions(options, imageObj, imageObjAlt) {
     this.options = options;
-    this.setImages(imageObj, imageObjAlt);
+
+    if (!(imageObj === undefined && imageObjAlt === undefined)) {
+      this.setImages(imageObj, imageObjAlt);
+    }
   }
 
+
+  /**
+   * Set the images for this node.
+   *
+   * The images can be updated after the initial setting of options;
+   * therefore, this method needs to be reentrant. 
+   *
+   * For correct working in error cases, it is necessary to properly set
+   * field 'nodes.brokenImage' in the options.
+   *
+   * @param {Image} imageObj  required; main image to show for this node
+   * @param {Image|undefined} optional; image to show when node is selected
+   */
   setImages(imageObj, imageObjAlt) {
     if (imageObjAlt && this.selected) {
       this.imageObj    = imageObjAlt;
@@ -38,17 +54,21 @@ class CircleImageBase extends NodeBase {
   }
 
   /**
-   * Switch between the base and the selected image.
+   * Set selection and switch between the base and the selected image.
+   *
+   * Do the switch only if imageObjAlt exists.
+   *
+   * @param {true|false} selected value of new selected state for current node
    */
   switchImages(selected) {
-    if ((selected && !this.selected) || (!selected && this.selected)) {
+    var selection_changed = ((selected && !this.selected) || (!selected && this.selected));
+    this.selected = selected;    // Remember new selection
+
+    if (this.imageObjAlt !== undefined && selection_changed) {
       let imageTmp = this.imageObj;
       this.imageObj = this.imageObjAlt;
       this.imageObjAlt = imageTmp;
     }
-
-    // keep current state in memory
-    this.selected = selected;
   }
 
   /**