Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

emergency, lightbox doesn't work on mobile #16

Closed
jasomdotnet opened this issue Dec 19, 2018 · 16 comments
Closed

emergency, lightbox doesn't work on mobile #16

jasomdotnet opened this issue Dec 19, 2018 · 16 comments

Comments

@jasomdotnet
Copy link

jasomdotnet commented Dec 19, 2018

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

@englishextra
Copy link
Owner

englishextra commented Dec 20, 2018

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
Copy link
Author

jasomdotnet commented Dec 20, 2018

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
Copy link
Owner

englishextra commented Dec 20, 2018

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
Copy link
Author

jasomdotnet commented Dec 20, 2018

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
Copy link
Owner

englishextra commented Dec 20, 2018

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
Copy link
Author

jasomdotnet commented Dec 20, 2018

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

  • as you wrote, iframe-lightbox.js must be in footer (before init)
  • there is conflict in touch-keyboard-navigation.js within enabled WP theme (Twenty Nineteen) with iframe-lightbox.js causing error on touch screen

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
Copy link
Owner

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
Copy link
Author

this wp demo site

@jasomdotnet
Copy link
Author

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
Copy link
Owner

englishextra commented Dec 20, 2018

@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
Copy link
Owner

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
Copy link
Author

I like that part about making me happy :-)

@jasomdotnet
Copy link
Author

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
Copy link
Owner

@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
Copy link
Owner

englishextra commented Dec 20, 2018

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants