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

just branding suggestion: consider rename to 'IFLI' #13

Closed
jasomdotnet opened this issue Dec 15, 2018 · 13 comments
Closed

just branding suggestion: consider rename to 'IFLI' #13

jasomdotnet opened this issue Dec 15, 2018 · 13 comments
Labels

Comments

@jasomdotnet
Copy link

Reconsider renaming to 'IFLI' IFrame LIghtbox. I personally like it more, but current name has god SEO (I googled it when I was looking for 'iframe lightbox').

In README.md you can add that the script is also:

  • request light - inlined svg right in css (only 2 additional request: 1. css + 1. js file)
  • retina (high DPI screens) ready - uses svg instead png
@englishextra
Copy link
Owner

@jasomdotnet Thanks for the suggestions and proposals.

IFLI is short but isnot obvious. There could be a name which is in no way connected to lightbox, say, Melina/Leeloo and it will work. But, yes, the SEO is much important.

As for inlined images and thus less request - very good idea. Thanks - will add that.
as for retina I will have to change CSS adding media query for retina and bringing SVG line there and keeping base64 for Edge and IE - that's what I am going to get over now. Many thanks.

@jasomdotnet
Copy link
Author

jasomdotnet commented Dec 15, 2018

as for retina I will have to change CSS adding media query for retina and bringing SVG line there and keeping base64 for Edge and IE - that's what I am going to get over now. Many thanks.

  1. SVG is retina ready because it is vector format (I know you know)
  2. Why you want to base64 encode svg for Edge or IE10? These (shitty) browsers can handle css with inlined SVGs (make sure width and height is defined - your SVGs have this defined). And if anybody comes with IE9 let the developers do a fix over extra css file using IE conditional tags:
<!--[if lte IE 9]>
 <link rel='stylesheet' id='old-ms-ie-fix'  href='https://cdn.jsdelivr.net/gh/englishextra/iframe-lightbox@master/old-ms-ie-fix.css' type='text/css' media='all' />
<![endif]-->
  1. Don't ruin your beautifull script because of Morkvosoft :)

@jasomdotnet
Copy link
Author

I just tested it in IE10 and Edge - I works perfect. If anybody wants backward compatibility with ancient (MS) browsers, let him use <!--[if lte IE 9]>.

@englishextra
Copy link
Owner

englishextra commented Dec 15, 2018

@jasomdotnet
Edge supports inlined urlencoded SVG - IE11 doesn't
Edge wont support inlined SVG with SMIL animation - the loader
Chrome and FF support SVG (closing button) as well as SMIL SVG (animated loader)

So what I need to do is

a) to detect with js IE and Edge and set classes to HTML elementand then in CSS serve the right image

what we have now is base64 encoded svg for close button and base64 encoded gif for loader

So I think how could I achieve that withCSS only approach

PS Oh yes I dont count anything less than IE11. That's right - the devs will manage. My concern is IE11 and Edge 13

Well I have to study this onece again and test https://stackoverflow.com/questions/10768451/inline-svg-in-css

@jasomdotnet
Copy link
Author

jasomdotnet commented Dec 15, 2018

@englishextra you are right, SVG animation doesn't work in IE11 (either). You have already inlined animated gif into your script, that's why it worked in IE10.

Alternative solution with svg (too keep "Retina readiness")

What about some static SVG + animate it using css animation or some javascript trick. Just as example see this (you may search for better example).

Consider this, because gif is too pixelate. Using GIF format sounds outdated to me.

https://css-tricks.com/animating-svg-css/

@englishextra
Copy link
Owner

thats what im working on it now thanks

@englishextra
Copy link
Owner

englishextra commented Dec 15, 2018

I finally used approach that I've already had before:

Check for smil support and add svganimate class to documentElement

So now base64 goes to Edge and IE11, and urlencoded svg to the rest of the browsers. That way the Retina Support is a real thing.

I'll wait a bit for your remarks, and soon will publish the v0.1.9 to NPM.

Thanks, you made me stand up and copy-paste some code.

@jasomdotnet
Copy link
Author

jasomdotnet commented Dec 15, 2018

I think you have decided to go wrong direction. The proof is increased size of source code (one of your script main advantages). Minified CSS has 60kb now! GIFs cannot be much gzipped. Now it's above level of compressed jQuery.

Don't do detection of SVG animation support (it only results in source code increase), leave in code only SVGs which you animate (rotate) using CSS for all browsers by default. In IE CSS animations are supported since IE9.

Ones again: Remove ugly-pixelate-big gif, remove detection of SVG animation support, add CSS animation default for all browsers, decrease source code size.

See this inspiration: https://codepen.io/abbeyjfitzgerald/pen/xddddL

@englishextra
Copy link
Owner

englishextra commented Dec 15, 2018

Oh yes - a pure CSS spinner can be used - then there'll be no smil check, but css for spinner (keyframes) and some html for spinner in JS - the advantage is that base64 images will be removed from css but instead the css with keyframes and the spinner itself will be added - in js the smil checl will be removed but inner html for spinner will be added But your suggestion sounds more cute and reasonable. And I already stopped using JS spinners in favour of these - https://github.com/epicmaxco/epic-spinners http://epic-spinners.epicmax.co/

This will go to 0.1.91

@jasomdotnet
Copy link
Author

jasomdotnet commented Dec 15, 2018

What a find! Epic-spinners is very adequate name :-)

Then I suggest go this css way completely. Here are pure-css-closing-icon samples:

http://jsfiddle.net/jermartin77/63sqmob6/
https://codepen.io/brissmyr/pen/egidw
https://codepen.io/ndeniche/pen/ljbDL

Tiniest code wins.

@englishextra
Copy link
Owner

OK - I know what I am doing next week.

@englishextra
Copy link
Owner

v0.2.0 on NPM https://www.npmjs.com/package/iframe-lightbox
999999 and pure CSS UI images

@jasomdotnet
Copy link
Author

great job!

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

No branches or pull requests

2 participants