Karmalakas / grav-plugin-photoswipe

Add Photoswipe gallery to your pages
MIT License
8 stars 1 forks source link

Incorrect thumbnail Y-offset calculation #6

Closed mkabakovitch closed 1 year ago

mkabakovitch commented 1 year ago

If the selector selector item is a, then the Y-offset is incorrectly calculated. Instead of using the top property of the selector item, the top property of enclosed img item must be used. Actually, there is no need for centerThumbPosition option. So, instead of current implementation:

        function getThumbnailBounds(index) {
            const
                thumbnail = gallery_items[index],
                pageYScroll = window.pageYOffset || document.documentElement.scrollTop,
                rect = thumbnail.getBoundingClientRect()
            ;

            let y = rect.top + pageYScroll;

            if (centerThumbPosition) {
                const thumbnail_image = thumbnail.querySelector('img');

                y += (rect.height / 2) - (
                    (
                        (thumbnail_image.width * thumbnail_image.naturalHeight) / thumbnail_image.naturalWidth
                    ) / 2
                );
            }

            return {x: rect.left, y, w: rect.width};
        }

Something like this could be used:

        function getThumbnailBounds(index) {
            const
                thumbnail = gallery_items[index],
                pageYScroll = window.pageYOffset || document.documentElement.scrollTop
            ;

            const thumbnail_image = thumbnail.querySelector('img');

            const rect = thumbnail_image.getBoundingClientRect();

            let y = rect.top + pageYScroll;

            return {x: rect.left, y, w: rect.width};
        }

Tested here

Karmalakas commented 1 year ago

I would like to avoid this solution, because some custom templates might use a background image on <a> and not have <img> at all.

Please try adding this CSS instead:

.gallery-item {display: inline-block;}
.gallery-item > img {display: block;}

This should make height of <a> same as <img> and calculations should work as expected (tested on your link with Dev Tools). If it's OK, I'll probably add a stylesheet to the plugin or update the README 🤔

mkabakovitch commented 1 year ago

The display soluton also works, so I reverted to the original version of the plugin.

I think, it would make sense at least to mention it in the documentation, because without this modification and with the template provided the offset would not be calculated correctly.