englishextra / iframe-lightbox

Responsive no-jQuery pure JS/CSS Lightbox for iframes, no dependencies, customizable aspect ratio, 5kb unminified source code, with demo
https://codepen.io/englishextra/pen/jmjayV
MIT License
28 stars 4 forks source link

emergency, lightbox doesn't work on mobile #16

Closed jasomdotnet closed 5 years ago

jasomdotnet commented 5 years ago

It opens the link. No pop-up is shown. See example

englishextra commented 5 years ago

update iframe-lightbox paths in your scripts test page for the latest v2.3.4 https://yuzhtushino.github.io/#/works

and codepen https://codepen.io/englishextra/pen/jmjayV also it is mentioned in changelog the library comes now with dev gulp setup so all scripts and styles have moved to css and js folders

https://github.com/englishextra/iframe-lightbox/blob/master/CHANGELOG.md

It seems that you had used dirict links to raw github, thus you'll need to update those paths.

iframe-lightbox.css -> css/iframe-lightbox.css
iframe-lightbox.js -> js/iframe-lightbox.js

0.2.4 - 2018-12-18 Changed Reorginized the file tree of the library

jasomdotnet commented 5 years ago

It seems that you had used dirict links to raw github, thus you'll need to update those paths.

I have noticed this : ) Here is a demo with latest gihhub js and css to see it's not working.

Test it using your mobile device.

englishextra commented 5 years ago

2018-12-20_125240_cr It seems that the script and style a served with the wrong mime type maybe try another platform for testing?

X-Content-Type-Options: nosniff I see in FF devtools for js and css TL;DR this boardea.com blocks content from cdn.jsdelivr.net I guess. May by try

https://unpkg.com/iframe-lightbox@latest/iframe-lightbox.js

https://unpkg.com/iframe-lightbox@latest/iframe-lightbox.css

or try

https://gitcdn.xyz/

If it fails, this is for sure that your testing platform blocks external scripts

If this platform is yours - maybe you have some default server config that sends no sniff to end user browser

Have you used that boardea.com platform for your previous testing and did that work?

jasomdotnet commented 5 years ago

Ok, demo is fixed and still with same problem. Firefox reports this:

TypeError: docBody[appendChild] is not a function[Learn More] iframe-lightbox.js:128:23

PS: shouldn't be this from README.md:

https://cdn.jsdelivr.net/gh/englishextra/iframe-lightbox@latest/iframe-lightbox.min.js
https://cdn.jsdelivr.net/gh/englishextra/iframe-lightbox@latest/iframe-lightbox.min.css
https://unpkg.com/iframe-lightbox@latest/iframe-lightbox.js
https://unpkg.com/iframe-lightbox@latest/iframe-lightbox.css

this:

https://cdn.jsdelivr.net/gh/englishextra/iframe-lightbox@latest/js/iframe-lightbox.min.js
https://cdn.jsdelivr.net/gh/englishextra/iframe-lightbox@latest/css/iframe-lightbox.min.css
https://unpkg.com/iframe-lightbox@latest/js/iframe-lightbox.js
https://unpkg.com/iframe-lightbox@latest/css/iframe-lightbox.css
englishextra commented 5 years ago

Yes the last ones. As for the rest I cant do anything - becasue as you can see the scripts are loaded bu the browser is tol not to execute - nosniff

Here it works https://codepen.io/englishextra/pen/jmjayV

So that's your server setup

Update - put the script from head to bottom of the body before init

        <script type="text/javascript" src="https://unpkg.com/iframe-lightbox@0.2.4/js/iframe-lightbox.js"></script>
        <script type="text/javascript">
            [].forEach.call(document.getElementsByClassName("iframe-lightbox-link"), function (el) {
                el.lightbox = new IframeLightbox(el, {
                    scrolling: true // default: false
                });
            });
        </script>

There is no waiting for window onload event. so the script doesn't see the body tag

It works now - I copied your demo on my desktop and tested that 2018-12-20_161633

Ok I just saw you fixed that.

There's a reson why there's no check for onload event - because the script is supposed to be included after the page loaded, placing a window onload inside the script may lead to bad consequences.

Or the script is bundled in some vendors.min.js that usually loaded with the script tag at the bottom

jasomdotnet commented 5 years ago

Most important is WordPress demo website, but anyway, this 'emergency' was combination of 2 reasons:

iframe-lightbox.js conflicts with javascript file touch-keyboard-navigation.js opening linked website instead opening a lightbox with linked website - it is touch screen alias mobile only.

englishextra commented 5 years ago

I uncluded that file to the demo - I see no conflicts

Can You specify what's wrong in dev console - what is it erroring about?

jasomdotnet commented 5 years ago

this wp demo site

jasomdotnet commented 5 years ago

Can You specify what's wrong in dev console - what is it erroring about? There is no error in dev console on my desktop browser. There is error on my mobile (I don't know how to open error console in Chrome on my android phone :-)

I have created you a simplified version of WP demo website (just html). You may play with that. Open it on your mobile phone and click on 'β' link (using your cell phone browser).

englishextra commented 5 years ago

@jasomdotnet touch-keyboard-navigation.js It's there:


    /**
     * Toggle `focus` class to allow sub-menu access on touch screens.
     */
    function toggleSubmenuDisplay() {

        document.addEventListener('touchstart', function(event) {

            if ( event.target.matches('a') ) {

                var url = event.target.getAttribute( 'href' ) ? event.target.getAttribute( 'href' ) : '';

                // If there’s a link, go to it on touchend
                if ( '#' !== url && '' !== url ) {
                    window.location = url;

This is there the clicks are hooked. This is something that you will have to resolve on your own.

Perhaps that was the reason why I used data-src instead of href, the other reason is that users normaly dont want to be moved to another site, because it's a lightbox not a regular external link

englishextra commented 5 years ago

OK Since other lightboxes also add touchstart I will add that too and this should do you happy, but first I have to test again

jasomdotnet commented 5 years ago

I like that part about making me happy :-)

jasomdotnet commented 5 years ago

We may back to data-src too. It works and I'm fine with that.

href is just in the case JS is turned off. I consider href to be more advanced.

englishextra commented 5 years ago

@jasomdotnet I already added touch support so the unceremonious touch-keyboard-navigation.js will not break the logic

href and data-src will be supported - no breaking change there

englishextra commented 5 years ago

published v0.2.5 here and on NPM

https://cdn.jsdelivr.net/gh/englishextra/iframe-lightbox@0.2.5/js/iframe-lightbox.min.js

https://cdn.jsdelivr.net/gh/englishextra/iframe-lightbox@0.2.5/css/iframe-lightbox.min.css

UPD Your boardea...test platform workson mobile as I just saw.

If problem persists, and you have access, try to make it that way:

    ...

<script type='text/javascript' src='........./wp-content/themes/twentynineteen/js/priority-menu.js?ver=1.0'></script>
<script type='text/javascript' src='........./wp-content/themes/twentynineteen/js/touch-keyboard-navigation.js?ver=1.0'></script>
<script type='text/javascript' src='........./wp-includes/js/wp-embed.min.js?ver=5.0.2'></script>
<!-- plugins are the last -->
<script type='text/javascript' src='........./wp-content/plugins/boardea-storyboard-integration/boardea.min.js?ver=1.6'></script>
    <script>
    ...
jasomdotnet commented 5 years ago

It works even when iframe-lightbox.js is not the last one. I'm feeling happy, thank you :) :)