Multiple Uppload instances have problems

[metalgigio]

Describe the bug
When having multiple istance of uppload (last versions but directly in browser) there are some issues on tab services, specially in the second istance.

To Reproduce
i made a demo in jsfidler: https://jsfiddle.net/40ntrcwf/
Try to click on "Change profile picture 2", select "choose file" and then try to switch to instagram upload.

Desktop:

• OS: Windows 10
• Browser Chrome
• Version 81

[AnandChowdhary]

Thanks for opening this issue, @metalgigio! I'll have a look and the problem. Maybe it's because of the direct browser usage (because it uses the global window object), or maybe it's an error with the package. I'll set up a CodeSandbox demo with multiple instances without using direct browser APIs and see if I can reproduce it there.

[metalgigio]

Thank you @AnandChowdhary ; let me know even if you find a temporary workaround.

[stormfar]

Is there any update on a fix/workaround for this? Having the same issue.

[stormfar]

This has been solved by #237. Thanks to @RichardJohnn!

[AnandChowdhary]

I can't believe that tiny mistake was causing this huge problem! ^.^

Fixed in 8936cc0. Big thanks!

[RichardJohnn]

This has been solved by #237.

I should say that I think this bug still remains an issue, but the workaround, @metalgigio , was to use one instance of uppload and then each time uppload is opened, remove the previous upload event listener before adding a new one to ensure just one callback fires.

[metalgigio]

Hi everyone, as @RichardJohnn said it does not solve the issue.
How can i workaround as you suggest?
That's my code if someone can help me:

var language = window.uppload_it;
        language.close              = "Chiudi";
        language.services.local.or  = "oppure";
        language.help.close         = "Chiudi Aiuto";
        
    
            
            var picture_foto_grande = new window.uppload_Uppload({
                call: ".uppload-foto_grande-button",
                bind: ".uppload-foto_grande-image",
                lang: language, 
                maxSize: [1280, 1280],
                maxFileSize: 25000,
                uploader: window.uppload_xhrUploader({
                    endpoint: "/ajax.php?uppload&c=foto_grande&t=cantine"
                })
            });

            picture_foto_grande.use([
              new window.uppload_Local(),
              new window.uppload_URL(),
              new window.uppload_Instagram(),
                new window.uppload_Crop({ aspectRatio: 16/9 })   
                
            ]);
            
            
            picture_foto_grande.on("upload", (url) => {
                var elems = document.querySelectorAll(".uppload-foto_grande-image");
                [].forEach.call(elems, function(el) {
                    if (!el.classList.contains('img-thumbnail')) {
                        el.classList.add("img-thumbnail");
                    }
                });
            });
            
            
            
            var picture_logo = new window.uppload_Uppload({
                call: ".uppload-logo-button",
                bind: ".uppload-logo-image",
                lang: language,
                maxSize: [1280, 1280],
                maxFileSize: 25000,
                uploader: window.uppload_xhrUploader({
                    endpoint: "/ajax.php?uppload&c=logo&t=cantine"
                })
            });

            picture_logo.use([
              new window.uppload_Local(),
              new window.uppload_URL(),
              new window.uppload_Instagram(),
                new window.uppload_Crop({ aspectRatio: 1/1 })
                
            ]);
            
            
            picture_logo.on("upload", (url) => {
                var elems = document.querySelectorAll(".uppload-logo-image");
                [].forEach.call(elems, function(el) {
                    if (!el.classList.contains('img-thumbnail')) {
                        el.classList.add("img-thumbnail");
                    }
                });
            });

[RichardJohnn]

you will need to make sure you have one new window.uppload_Uppload call.

in my case I had a button with an onClick to handle opening the model myself, but before doing that i did a picture_logo.off("upload", [...] of the original handler before doing the .on so you control what the callback is.

maybe you have one callback where you just pass in your selector like ".uppload-foto_grande-image" vs ".uppload-logo-image"

[metalgigio]

Thank you for ur suggest @RichardJohnn , i will try as soon as i can.

[metalgigio]

hi,
i solved using ur suggestion, here's my code if someone need similar workaround:

const myUploader = new window.uppload_Uppload({
            lang: language,
            maxSize: [1280, 1280],
            maxFileSize: 25000,
        });
        
    
            
        document.addEventListener('DOMContentLoaded', function() {
        
            
            
            $('.uppload-foto_grande-button').on('click',function(){
                
                myUploader.updateSettings({
                    bind: ".uppload-foto_grande-image",
                    uploader: window.uppload_xhrUploader({
                        endpoint: "/ajax.php?uppload&c=foto_grande&t=cantine"
                    })
                });
                
                myUploader.updatePlugins(plugins => []);
                
                myUploader.use([
                    new window.uppload_Local(),
                    new window.uppload_URL(),
                    new window.uppload_Instagram(),
                        new window.uppload_Crop({ aspectRatio: 16/9 })
                    
                ]);
                
                myUploader.off("upload");
                
                myUploader.on("upload", (url) => {
                    var elems = document.querySelectorAll(".uppload-foto_grande-image");
                        [].forEach.call(elems, function(el) {
                            if (!el.classList.contains('img-thumbnail')) {
                            el.classList.add("img-thumbnail");
                        }
                    });
                });
                
                myUploader.open();


            });    
            
            
            $('.uppload-logo-button').on('click',function(){
                
                myUploader.updateSettings({
                    bind: ".uppload-logo-image",
                    uploader: window.uppload_xhrUploader({
                        endpoint: "/ajax.php?uppload&c=logo&t=cantine"
                    })
                });
                
                myUploader.updatePlugins(plugins => []);
                
                myUploader.use([
                    new window.uppload_Local(),
                    new window.uppload_URL(),
                    new window.uppload_Instagram(),
                        new window.uppload_Crop({ aspectRatio: 1/1 })
                    
                ]);
                
                myUploader.off("upload");
                
                myUploader.on("upload", (url) => {
                    var elems = document.querySelectorAll(".uppload-logo-image");
                        [].forEach.call(elems, function(el) {
                            if (!el.classList.contains('img-thumbnail')) {
                            el.classList.add("img-thumbnail");
                        }
                    });
                });
                
                myUploader.open();

            });
            
        });

[phlegx]

The suggested workarounds are not so elegant. Investigating on this problem I have found the bug(s).

1. Working with HTML input tag ids is tricky and the ids should be unique in DOM.
2. Event listener for "escape" should also run right on multiple instances.

So, I introduced:

1. the setting "id". This can be used to set an unique id to the container and related sub tags. If not set, an unique id is created automatically per instance.
2. the function safeUnlisten to be able to remove a safe added listener to solve the escape bug.

And here the code:

// src/helpers/elements.ts add after the safeListen export.

/**
 * Safely remove an event listener
 * @param element - HTML element to remove event listener to
 * @param type - Type of event listener to remove
 * @param fn - Callback function to associated listener
 */
export const safeUnlisten = (
  element: Element,
  type: string,
  fn: EventListenerOrEventListenerObject
) => {
  const listener = listening.findIndex((a) => a.element === element && a.type === type);
  if (listener < 0) return;
  element.removeEventListener(type, fn);
  listening.splice(listener, 1);
};

// src/uppload.ts

import { getElements, safeListen, safeUnlisten, compressImage } from "./helpers/elements";
...
export class Uppload implements IUppload {
  id: string = `${+new Date()}`;
  ...
}

constructor(settings?: IUpploadSettings) {
  this.settings = {};
  this.updateSettings(settings || {});
  this.container = document.createElement("div");
  this.container.setAttribute("id", `uppload-${this.id}`);
  ...
}

updateSettings(settings: IUpploadSettings) {
  this.settings = { ...this.settings, ...settings };
  this.emitter.emit("settingsUpdated", settings);
  if (settings.id) this.id = settings.id;
  ...
}

open() {
  ...
  if (firstService) this.navigate(firstService);
  // Replace safeListen call with:
  const escape = (e) => {
    if (e.key === "Escape" && this.open) {
      this.close();
    }
  };
  safeUnlisten(document.body, "keyup", escape);
  safeListen(document.body, "keyup", escape);
  ...
}

getNavbar(sidebar = false) {
  ...
  // Add `this.id` for radio and label.
  `<input type="radio" id="uppload-service-radio-${service.name}-${this.id}" ...`
  ...
  `label for="uppload-service-radio-${service.name}-${this.id}"`
  ...
}

getEffectsNavbar() {
  ...
  // Add `this.id` for radio and label.
  `<input type="radio" id="uppload-effect-radio-${effect.name}-${this.id}" ...`
  `<label for="uppload-effect-radio-${effect.name}-${this.id}">`
  ...
}

I have found more bugs and I will commit the changes in a PR.

Another open issue is if this.open should be replace by this.isOpen in:

const escape = (e) => {
  if (e.key === "Escape" && this.open) {
    this.close();
  }
};

because this.open is only the reference to the open() function.