Get data ids and not number of data, fixes #1424

Avoids relying on consecutive data ids. Add check in render.
ivmartel · Jul 4, 2023 · 90f1008 · 90f1008
1 parent 49d58de
commit 90f1008
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 16 deletions.
diff --git a/src/app/application.js b/src/app/application.js
@@ -88,7 +88,8 @@ export class App {
    * @returns {Image} The image.
    */
   getLastImage() {
-    return this.#dataController.get(this.#dataController.length() - 1).image;
+    const dataIds = this.#dataController.getDataIds();
+    return this.#dataController.get(dataIds[dataIds.length - 1]).image;
   }
 
   /**
@@ -107,7 +108,8 @@ export class App {
    * @param {Image} img The associated image.
    */
   setLastImage(img) {
-    this.#dataController.setImage(this.#dataController.length() - 1, img);
+    const dataIds = this.#dataController.getDataIds();
+    this.#dataController.setImage(dataIds[dataIds.length - 1], img);
   }
 
   /**
@@ -118,7 +120,7 @@ export class App {
    * @returns {number} The new image id.
    */
   addNewImage(image, meta) {
-    const id = this.#dataController.length();
+    const id = this.#dataController.getDataIds().length;
 
     // load start event
     this.#fireEvent({
@@ -174,12 +176,12 @@ export class App {
   }
 
   /**
-   * Get the number of loaded data.
+   * Get the list of ids in the data storage.
    *
-   * @returns {number} The number.
+   * @returns {Array} The list of data ids.
    */
-  getNumberOfLoadedData() {
-    return this.#dataController.length();
+  getDataIds() {
+    return this.#dataController.getDataIds();
   }
 
   /**
@@ -675,7 +677,7 @@ export class App {
     if (itemIndex === -1) {
       this.#options.dataViewConfigs[dataId].push(config);
     } else {
-      throw new Error('Duplicate view sconfig for data ' + dataId +
+      throw new Error('Duplicate view config for data ' + dataId +
         ' and div ' + config.divId);
     }
 
@@ -828,7 +830,8 @@ export class App {
       }
       // add view
       // warn: needs a loaded DOM
-      if (layerGroup.getViewLayersByDataIndex(dataIndex).length === 0) {
+      if (typeof this.#dataController.get(dataIndex) !== 'undefined' &&
+        layerGroup.getViewLayersByDataIndex(dataIndex).length === 0) {
         this.#addViewLayer(dataIndex, config);
       }
       // draw

diff --git a/src/app/dataController.js b/src/app/dataController.js
@@ -26,12 +26,12 @@ export class DataController {
   #listenerHandler = new ListenerHandler();
 
   /**
-   * Get the length of the data storage.
+   * Get the list of ids in the data storage.
    *
-   * @returns {number} The length.
+   * @returns {Array} The list of data ids.
    */
-  length() {
-    return Object.keys(this.#data).length;
+  getDataIds() {
+    return Object.keys(this.#data);
   }
 
   /**

diff --git a/tests/pacs/viewer.js b/tests/pacs/viewer.js
@@ -327,10 +327,11 @@ function onDOMContentLoaded() {
     _app.setDataViewConfig(configs);
 
     clearDataTable();
-    for (let i = 0; i < _app.getNumberOfLoadedData(); ++i) {
-      _app.render(i);
+    const dataIds = _app.getDataIds();
+    for (let i = 0; i < dataIds.length; ++i) {
+      _app.render(dataIds[i]);
       // add data row (will bind controls)
-      addDataRow(i);
+      addDataRow(dataIds[i]);
     }
 
     // need to set tool after config change