Plugin lgZoom - little mistakes in the HTML/JS code

Athar42 commented 10 months ago

Description

Hi there,

A few mistakes in the code I saw so far while moving from v1 to v2 :

In the JS part first : ==> If "showZoomInOutIcons" is enable, the declaration for the HTML code of the "Zoom out" button is wrongly set :

var zoomIcons = this.settings.showZoomInOutIcons
                ? "<button id=\"" + this.core.getIdName('lg-zoom-in') + "\" type=\"button\" aria-label=\"" + this.settings.zoomPluginStrings['zoomIn'] + "\" class=\"lg-zoom-in lg-icon\"></button><button id=\"" + this.core.getIdName('lg-zoom-out') + "\" type=\"button\" aria-label=\"" + this.settings.zoomPluginStrings['zoomIn'] + "\" class=\"lg-zoom-out lg-icon\"></button>"

Should be (second button) :

var zoomIcons = this.settings.showZoomInOutIcons
                ? "<button id=\"" + this.core.getIdName('lg-zoom-in') + "\" type=\"button\" aria-label=\"" + this.settings.zoomPluginStrings['zoomIn'] + "\" class=\"lg-zoom-in lg-icon\"></button><button id=\"" + this.core.getIdName('lg-zoom-out') + "\" type=\"button\" aria-label=\"" + this.settings.zoomPluginStrings['zoomOut'] + "\" class=\"lg-zoom-out lg-icon\"></button>"

Still in that file, just under this code, we have the definition of the "actualSize" :

if (this.settings.actualSize) {
                zoomIcons += "<button id=\"" + this.core.getIdName('lg-actual-size') + "\" type=\"button\" aria-label=\"" + this.settings.zoomPluginStrings['viewActualSize'] + "\" class=\"" + this.settings.actualSizeIcons.zoomIn + " lg-icon\"></button>";
            }

Should be refering to "lg-actual-size" icon, but not declared at first. If this is intended (change from v1 to v2), that's an issue as both ZoomIn/ZoomOut used the same icon set and lead to confusion : So, maybe do those two changes :

if (this.settings.actualSize) {
                zoomIcons += "<button id=\"" + this.core.getIdName('lg-actual-size') + "\" type=\"button\" aria-label=\"" + this.settings.zoomPluginStrings['viewActualSize'] + "\" class=\"" + this.settings.actualSizeIcons.viewActualSize + " lg-icon\"></button>";
            }

and from :

var zoomSettings = {
        scale: 1,
        zoom: true,
        infiniteZoom: true,
        actualSize: true,
        showZoomInOutIcons: false,
        actualSizeIcons: {
            zoomIn: 'lg-zoom-in',
            zoomOut: 'lg-zoom-out',
        },
        enableZoomAfter: 300,
        zoomPluginStrings: {
            zoomIn: 'Zoom in',
            zoomOut: 'Zoom out',
            viewActualSize: 'View actual size',
        },
    };

to :

var zoomSettings = {
        scale: 1,
        zoom: true,
        infiniteZoom: true,
        actualSize: true,
        showZoomInOutIcons: false,
        actualSizeIcons: {
            zoomIn: 'lg-zoom-in',
            zoomOut: 'lg-zoom-out',
            viewActualSize: 'lg-actual-size',
        },
        enableZoomAfter: 300,
        zoomPluginStrings: {
            zoomIn: 'Zoom in',
            zoomOut: 'Zoom out',
            viewActualSize: 'View actual size',
        },
    };

I didn't checked all the code, but might be some more references to those icons, so maybe some more line of code would need to be replaced too :)

Also, a little bug, when we do a zoom in (infinite zoom), then we click on the "acual size" to revert to the original size of the picture, we get those three icons greyed out (Actual Size + Zoom In + Zoom Out). To get them working again, a double click on the image reset the buttons.

Edit : About this little bug, my assumption is on the CSS side, here :

.lg-actual-size .lg-icon.lg-zoom-in {
  opacity: 0.5;
  pointer-events: none;
}

When it's reseted, it called this and the "pointer-events" is set to none which disable the two button. Either setting this to auto or remove those lines would fix it (don't know yet if it's triggered for something else)

Steps to reproduce

This icon behavior (not ZoomIn/ZoomOut) can be seen on this Demo page : https://www.lightgalleryjs.com/demos/thumbnails

Environment

Browser and version - Firefox 121.0.1
OS - Windows 11 23H2
lightGallery version - 2.7.2

Additional context

bytesandbots3 commented 9 months ago

Thank you for bringing these issues to our attention.

Incorrect aria-label for Zoom Out Button: The aria-label for the "Zoom out" button has been corrected. It now appropriately reflects zoomOut instead of zoomIn. and

Greyed Out Icons Issue that you mentioned.

stale[bot] commented 7 months ago

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.

sachinchoolur / lightGallery