lightGallery icon indicating copy to clipboard operation
lightGallery copied to clipboard

Error triggered when opening gallery on 2.2.1

Open dwightwatson opened this issue 3 years ago • 28 comments

Description

Cannot open the gallery on the initial load.

Clicking on an image, or a button that is wired to open the gallery causes an error:

Error invoking action "click->image-gallery#openImages"

TypeError: Cannot read properties of null (reading 'focus')
    at LightGallery.push../node_modules/lightgallery/lightgallery.es5.js.LightGallery.openGallery (vendors-node_modules_algolia_autocomplete-js_dist_esm_autocomplete_js-node_modules_algolia_au-7a256d.js:27553)
    at extended.openImages (application.js:2556)
    at Binding.push../node_modules/@stimulus/core/dist/binding.js.Binding.invokeWithEvent (vendors-node_modules_algolia_autocomplete-js_dist_esm_autocomplete_js-node_modules_algolia_au-7a256d.js:12888)
    at Binding.push../node_modules/@stimulus/core/dist/binding.js.Binding.handleEvent (vendors-node_modules_algolia_autocomplete-js_dist_esm_autocomplete_js-node_modules_algolia_au-7a256d.js:12865)
    at EventListener.push../node_modules/@stimulus/core/dist/event_listener.js.EventListener.handleEvent (vendors-node_modules_algolia_autocomplete-js_dist_esm_autocomplete_js-node_modules_algolia_au-7a256d.js:13680)

Oddly, refreshing the page then fixes the issue. However, the issue re-appears for each new page.

~~This error appeared in 2.2.1 and is resolved by downgrading back to 2.2.0~~. Edit: this error is present in at least 2.2.0 and 2.2.1.

Steps to reproduce

For context, this is a Rails app using Stimulus. We have a license key and also use the video plugin. Here is the Stimulus controller (if helpful):

import lightGallery from "lightgallery";
import lgVideo from "lightgallery/plugins/video";
import "lightgallery/css/lightgallery-bundle.css";
import { Controller } from "stimulus";
import { LIGHTGALLERY_LICENSE_KEY } from "../constants";

export default class extends Controller {
  static targets = ["gallery", "image"];

  connect() {
    this.gallery = lightGallery(this.galleryTarget, {
      licenseKey: LIGHTGALLERY_LICENSE_KEY,
      plugins: [lgVideo],
      download: false,
    });
  }

  disconnect() {
    this.gallery?.destroy();
  }

  openImages(event) {
    event.preventDefault();

    this.gallery.openGallery();
  }

  openVideo(event) {
    event.preventDefault();

    this.gallery.openGallery(0);

    this.gallery.goToPrevSlide();
  }
}

Expected result

The gallery should just open - either when triggered by clicking an image, or calling openGallery().

Actual result

The console logs the error: TypeError: Cannot read properties of null (reading 'focus')

Which appears to be caused by this.outer.get() returning null.

    LightGallery.prototype.openGallery = function (index, element) {
        var _this = this;
        if (index === void 0) { index = this.settings.index; }
        // prevent accidental double execution
        if (this.lgOpened)
            return;
        this.lgOpened = true;
        this.outer.get().focus();

        ....

Additional context

dwightwatson avatar Sep 12 '21 23:09 dwightwatson

Hey fellow Stimulus-er... Are you using Turbo as well? I don't know if the following solves your problem, but I had to do the following to integrate it properly, so maybe it'll help someone else too.

class extends Controller {
  connect () {
    // if loading from Turbo cache, don't bother starting up LG
    if (document.documentElement.hasAttribute("data-turbo-preview")) {
      return 
    }

    // lg.destroy() is async, so does not manage to clean up before the DOM is cached by Turbo
    // so, we have to clean up ourselves
    document.querySelectorAll(".lg-container").forEach(e => e.parentNode.removeChild(e))

    // now, we can start LG
    this.gallery = lightGallery(...)
  }

  disconnect () {
    this.gallery?.destroy()
  }
}

vwong avatar Sep 13 '21 02:09 vwong

Thanks Voon - yeah we're running Turbo too. That's a good tip, I'll have a go running that code.

Did you just come across the issue on 2.2.1 as well, or had you spotted it earlier?

Edit: Wonder if you could just get away with document.querySelectorAll('.lg-container').forEach(element => element.remove()).

dwightwatson avatar Sep 13 '21 02:09 dwightwatson

No, mine is a different problem. Started from 2.2.0. https://github.com/sachinchoolur/lightGallery/issues/1149

vwong avatar Sep 13 '21 03:09 vwong

Hey @dwightwatson,

2.2.1 release has only one bug fix related to zoom plugin.

I think the issue here is that openImages is getting called before lightGallery is initialized or it is using a different instance

I don't have much experience with Rails app. Please make sure that openImages uses the correct lightGallery instance. And in case if connect gets called multiple times, please make sure that the previous lightGallery instance is already destroyed.

openImages(event) {
    event.preventDefault();
    if(!this.gallery) return
    this.gallery.openGallery();
  }

I think @vwong's solution is also on the same line. But, instead of removing .lg-container yourself, you can just wait for lightGallery to complete destroy process

destroy method return the time takes to destroy the gallery instance completely.


class extends Controller {
  connect () {
    // if loading from Turbo cache, don't bother starting up LG
    if (document.documentElement.hasAttribute("data-turbo-preview")) {
      return 
    }

    if(this.gallery) {
        const timeToDestroy = this.gallery?.destroy();
        setTimeout(()=>{
            this.gallery = lightGallery(...)
        },timeToDestroy );
    } else {
        // now, we can start LG
        this.gallery = lightGallery(...)
    }
    
  }

  disconnect () {
    this.gallery?.destroy()
  }
}

Let me know if it resolves the issue

sachinchoolur avatar Sep 13 '21 03:09 sachinchoolur

If you do go down this path, be very careful with setTimeout without a corresponding clearTimeout in case you need to cancel it if/when you have multiples; you can easily end up with a scenario whereby you can have function being invoked when you weren't expecting it.

A cleaner API would be for gallery.destroy() to return a promise, so that it could be awaited on. (Or have a callback when done).

vwong avatar Sep 13 '21 05:09 vwong

Yes, I was planning to add a callback function as IE doesn't support promise. That's why the timeout on destroy method is not included in the documentation

sachinchoolur avatar Sep 13 '21 05:09 sachinchoolur

Alright, I can see this is present in 2.2.0 as well.

What's more - this error occurs when clicking the image (i.e. not just when I attempt to trigger it with openGallery). If clicking the image is attempting to open the gallery under the hood then it seems that the lightGallery instance would already have been initialized (unless I'm mistaken?).

While Stimulus could call connect multiple times in the same instance (state isn't shared between instances) it doesn't seem to be happening here in a way that could be causing the issue. this.gallery is always missing in my testing here.

connect() {
 if (this.gallery) {
    return console.error("gallery already exists?");
  }

  //
}

Might also be worth pointing out that this doesn't appear to be a speed issue like #1149. I can keep the keep the page open for a couple of second and trigger the error.

dwightwatson avatar Sep 13 '21 05:09 dwightwatson

Could you please share your website URL if possible? I think I've experienced a similar error in the past when I was using lightGallery with React. The issue was due to multiple renderings.

sachinchoolur avatar Sep 13 '21 06:09 sachinchoolur

Sure - I'm looking to downgrade to 2.1.8 but I'll hold off in the meantime. Still got 2.2.0 in production.

We have published listings here: https://www.myrent.co.nz/listings when you click through to a listing there is an image gallery across the top. Clicking an image normally opens the gallery (which I understand is default behaviour) bu clicking the camera icon at the bottom left of the gallery should also open the gallery.

Let me know if this is helpful or if there is more I can provide.

dwightwatson avatar Sep 13 '21 06:09 dwightwatson

I am also using turbolinks (I use stimulus but I don't use it directly with lightgallery) and I have a similar issue. Everything works perfectly when I refresh the page, but on initial load I get this error:

Uncaught TypeError: Cannot read properties of null (reading 'focus')
    at LightGallery../node_modules/lightgallery/lightgallery.es5.js.LightGallery.openGallery (lightgallery.es5.js:1143)
    at HTMLAnchorElement.<anonymous> (lightgallery.es5.js:854)

which seems to point to:

# ligthgallery.es5.js

this.outer.get().focus();

This is my code:

# packs/application.js

import "lightgallery/scss/lightgallery-bundle.scss";
import { initLightbox } from '../plugins/lightbox';

document.addEventListener('turbolinks:load', () => {
  initLightbox();
});
# plugins/lightbox.js

import lightGallery from 'lightgallery';
import lgVideo from 'lightgallery/plugins/video'

const initLightbox = () => {
  const galleryContainer = document.getElementById('lightgallery');
  if (galleryContainer) {
    lightGallery(galleryContainer, {
      selector: '.lightgallery-item',
      plugins: [lgVideo],
    });
  }
}

export { initLightbox };

# views/application.html.erb

<head>
...
    <%= stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track': 'reload' %>
    <%= javascript_pack_tag 'application', 'data-turbolinks-track': 'reload', defer: true %>
</head>

I am using [email protected] but I also tried with 2.2.0 and 2.1.8 and got the same behaviour.

Edit : oddly, setting a container makes it work on first load (but acts more as an inline gallery which is not what I want here).

    lightGallery(galleryContainer, {
      selector: '.lightgallery-item',
      container: 'body',
      plugins: [lgVideo],
    });

alexplatteeuw avatar Sep 16 '21 11:09 alexplatteeuw

@ alexplatteeuw is that using Turbo Drive or Turbolinks? Be curious to note if this is specific to Turbo or not.

It sounds like something in the Turbo(links?) page transition is causing an issue with lightGallery...

dwightwatson avatar Sep 17 '21 04:09 dwightwatson

It looks like Turbolinks, from his example.

<%= javascript_pack_tag 'application', 'data-turbolinks-track': 'reload', defer: true %>

The equivalent in Turbo would be data-turbo-track.

vwong avatar Sep 17 '21 04:09 vwong

I had a similar issue, which I solved with https://github.com/sachinchoolur/lightGallery/issues/1149#issuecomment-917846872

I don't have your particular problem, but I tried the following very similar strategy to patch your site @dwightwatson https://www.myrent.co.nz/listings, and it works fine thereafter. Maybe consider that a near-term hack until the underlying problem is solved?

--- node_modules/lightgallery/lightgallery.es5.js	1985-10-26 18:15:00.000000000 +1000
+++ vendor/patches/lightgallery.es5.js	2021-09-17 15:24:39.738680370 +1000
@@ -1140,6 +1140,8 @@
         if (this.lgOpened)
             return;
         this.lgOpened = true;
+        if (!this.outer)
+            return;
         this.outer.get().focus();
         this.outer.removeClass('lg-hide-items');
         // Add display block, but still has opacity 0

vwong avatar Sep 17 '21 05:09 vwong

@dwightwatson : it is turbolinks ;)

I ended up using Lightbox - stimulus components which uses lightgallery.js (v. 1.4.0) behind the scene (I guess lightgallery.js is equivalent to lightgallery but without the plugins included) . Then I added lg-video.js independently:

import Lightbox from "stimulus-lightbox"
import lgVideo from "lg-video.js/dist/lg-video.js"
import "lightgallery.js/dist/css/lightgallery.min.css"

export default class extends Lightbox {
  connect() {
    super.connect()

    // Default options for every lightboxes.
    this.defaultOptions

    // The lightGallery instance.
    this.lightGallery
  }

  // You can set default options in this getter.
  get defaultOptions () {
    return {
      selector: '.lightgallery-item',
      plugins: [lgVideo]
    }
  }
}

And this does work well together with turbolinks. I will give a try to the patch above and edit this post to let you know.

alexplatteeuw avatar Sep 17 '21 07:09 alexplatteeuw

Just note that the patch just prevents the code from crashing, it doesn't actually fix the bug and open the gallery.

Hopefully can find an actual resolution to this soon, otherwise we'll probably roll back to the older version too.

dwightwatson avatar Sep 17 '21 07:09 dwightwatson

Also seeing this issue with Turbolinks. I have rolled back to v1 for now.

boboldehampsink avatar Sep 20 '21 11:09 boboldehampsink

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Nov 19 '21 11:11 stale[bot]

bump @sachinchoolur

boboldehampsink avatar Nov 19 '21 11:11 boboldehampsink

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jan 18 '22 12:01 stale[bot]

bump @sachinchoolur

boboldehampsink avatar Jan 18 '22 12:01 boboldehampsink

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Mar 19 '22 12:03 stale[bot]

bump @sachinchoolur

dwightwatson avatar Mar 20 '22 14:03 dwightwatson

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar May 19 '22 17:05 stale[bot]

bump

dwightwatson avatar May 19 '22 22:05 dwightwatson

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 19 '22 01:07 stale[bot]

bump

dwightwatson avatar Jul 19 '22 03:07 dwightwatson

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 17 '22 04:09 stale[bot]

bump 🥲

dwightwatson avatar Sep 17 '22 04:09 dwightwatson

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Nov 16 '22 07:11 stale[bot]

one year of bumping

boboldehampsink avatar Nov 16 '22 07:11 boboldehampsink