bnomei / kirby3-srcset

Kirby 3 Plugin for creating lazyloading image srcset
https://forum.getkirby.com/t/kirby-3-srcset-lazyloading-image-srcset-element/23575
MIT License
43 stars 3 forks source link

Page breaks when Editor plugin is not installed #21

Closed texnixe closed 4 years ago

texnixe commented 4 years ago

As soon as I install the plugin into a new Kirby installation that doesn't have the Editor installed, all I get when opening the site in a browser is a blank page, no error messages at all.

colin-johnston commented 4 years ago

I have the same issue.

Alternately, if I do have the Editor installed, using the lazysrcset Kirby Tag in a standard textarea does not render an image.

bnomei commented 4 years ago

https://github.com/bnomei/kirby3-srcset/releases/tag/v3.3.2 should fix that

bnomei commented 4 years ago

I have the same issue.

✅ fixed in 3.3.2

Alternately, if I do have the Editor installed, using the lazysrcset Kirby Tag in a standard textarea does not render an image.

👁 please verify that the image does also not appear in html src – it should be there. this plugin strongly optimized or the js lazysizes lib. it provides no default src for example. so unless you have lazysizes and proper css you might simply not see anything (but html code is there).

colin-johnston commented 4 years ago

The image does not appear in html src, only an inline style tag.

When I add the image as shown in screenshot here, the source renders as shown below.

image

// ...
<h1>About Me</h1>
    <p>Test</p>
    <style>.lazysrcset-ratio[data-ratio="61.8"]{padding-bottom:61.8%;}</style>

I had added this to my <head> just before script srcs:

<style>
    /* lazysrcset */
    figure { width: 100%; }
    img[data-sizes="auto"] { display: block; width: 100%; }
</style>

and I have this added to my existing _lazyload.scss partial (aleady had lazysizes involved):

.lazysrcset-ratio {  
    position: relative;
}
.lazysrcset-ratio > img {
    display: block;    
    width: 100%;
    height: 100%;
    position: absolute;
    top: 0;
    left: 0;
}
.lazysrcset-ratio > img:after {
    display: block;
    width: 100%;
    height: 0;
    content: '';
}

Please let me know if it looks like I'm doing some thing wrong. Hope this info helps.

bnomei commented 4 years ago

since the style tag is there with a proper ratio is very hard for me to understand why the rest should not work. you could at around these lines add a var_dump($this-text); die; for each apply step. This way we can find out what hapend after each step.

https://github.com/bnomei/kirby3-srcset/blob/master/classes/Bnomei/Srcset.php#L57

        $this->text = $this->imageKirbytagFromData($this->options);
var_dump($this->text);
        $this->text = $this->applySrcset($this->text, $this->options);
var_dump($this->text);
        $this->text = $this->applyRatio($this->text, $this->options);
var_dump($this->text); die;
colin-johnston commented 4 years ago

Yes, I agree with your thought—since the style tag was printing, there was at least some 'awareness' of an image, but odd that there's no img tag printing though.

When I add those lines to the Srcset class, the kirbytext prints as a string.

Screenshot 2020-03-02 18 30 07 Screenshot 2020-03-02 18 29 37
colin-johnston commented 4 years ago

I have a very basic customization for generating srcset in kirbytext that I'm going to work with right now, but I'm happy to do more testing/troubleshooting on this if it helps you. Just let me know!

When I have a project where I'll use the Editor I'll definitely install this though.

bnomei commented 4 years ago

please use $this->text with ->.

colin-johnston commented 4 years ago

Oh wow, I didn’t even notice that—I’ll update and try again this afternoon. On Mar 4, 2020, 4:05 AM -0800, Bruno Meilick notifications@github.com, wrote:

please use $this->text with ->. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

bnomei commented 4 years ago

Sorry was my fault. i typed that to hastily. 😅

colin-johnston commented 4 years ago

I definitely just pasted it in without even looking past the first dump 🤦‍♂️ I need to take it more slo mo On Mar 4, 2020, 9:46 AM -0800, Bruno Meilick notifications@github.com, wrote:

Sorry was my fault. i typed that to hastily. 😅 — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

colin-johnston commented 4 years ago

OK, so I fixed dump and no I'm seeing:

<h1>About Me</h1>

string(112) "

<figure class="lazysrcset-ratio">
<img alt="" class=" lazyloaded" 
src="https://srcset.net/placeholder.jpg">
</figure>

" string(533) "

<figure class="lazysrcset-ratio">
<img alt="" 
class="lazyautosizes lazyloaded" 
data-src="//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-q80.jpg" 
data-srcset="//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-300x.jpg 300w, 
//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-800x.jpg 800w, 
//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-1024x.jpg 1024w" 
data-thumb-srcset="" 
data-sizes="auto" sizes="777px" 
srcset="//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-300x.jpg 300w, 
//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-800x.jpg 800w, 
//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-1024x.jpg 1024w" 
src="//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-q80.jpg">
</figure>

" string(625) "

<style>.lazysrcset-ratio[data-ratio="61.8"]{padding-bottom:61.8%;}</style>

<figure data-ratio="61.8" class="lazysrcset-ratio">
<img alt="" class="lazyautosizes lazyloaded" 
data-src="//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-q80.jpg" 
data-srcset="//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-300x.jpg 300w, 
//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-800x.jpg 800w, 
//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-1024x.jpg 1024w" 
data-thumb-srcset="" 
data-sizes="auto" sizes="777px" 
srcset="//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-300x.jpg 300w, 
//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-800x.jpg 800w, 
//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-1024x.jpg 1024w" 
src="//localhost:3000/media/pages/about/4290059797-1583359893/colin-connect-2018-q80.jpg">
</figure> 
"

The last of the three <figure>s renders an image to the page.

bnomei commented 4 years ago

i am not sure where classes lazyautosizes lazyloaded come from. the default class of plugin is named lazyload and that matches the lazysizes default.

did you overwrite that in a config? are you using a different lazy loader lib?

colin-johnston commented 4 years ago

I'm using lazysizes@4.1.8 library, with no modifications.

Using this in my Kirby config:

'thumbs' => [
    'srcsets' => [
      'default' => [400, 800, 1200, 1600],
      'cover' => [800, 1024, 2048],
    ],
  ]

... but I'm not calling these configs in the Kirbytext, i.e. (lazysrcset: colin-connect-2018.jpg)

Those classnames are coming from the library, i.e. https://github.com/aFarkas/lazysizes/blob/a2f37ec2371bff523ea8b94800682e4ec3596b9b/lazysizes.js#L19 and https://github.com/aFarkas/lazysizes/blob/a2f37ec2371bff523ea8b94800682e4ec3596b9b/lazysizes.js#L24

In my current site using lazysizes, the initially set lazyload class is toggled to lazyloaded once the element is loaded. I believe it's working the same way here, and that's why there's no lazyload classname visible in the output—the elements are visible and have rendered by the time I grabbed this output.

Hope this helps :)

colin-johnston commented 4 years ago

A couple of quick notes.

Missing elements after lazysrcset kirbytext

If I use (image: ..., like this

image

I get this

<main id="content" class="o-content o-layout__grid-item c-page">
<h1 class="c-page__title">About Me</h1>
<p>Test</p>
<p>Maybe1</p>
<figure><img alt="Colin and Conor" src="http://colinjohnston.local/media/pages/about/4290059797-1583363454/colin-connect-2018.jpg"><figcaption>Test Caption</figcaption></figure>
<p>Maybe2</p>
</main>

When I use (lazysrcset: ... like this

image

I get this

<main id="content" class="o-content o-layout__grid-item c-page">
<h1 class="c-page__title">About Me</h1>
<p>Test</p>
<p>Maybe1</p>
<style>.lazysrcset-ratio[data-ratio="61.8"]{padding-bottom:61.8%;}</style>
</main>

This shows that in the case of lazysrcset nothing after the kirbytext entry renders in html i.e. nothing after the missing <figure>...</figure> renders until the end of the parent <main> so there's no <p>Maybe2</p> in second example.

Commenting out applyRatio line at end of constructor

If I comment out L58 in Srcset class, the image renders with complete srcset code, but of course is missing the <style> tag with ratio.

I hope some of this helps in troubleshooting!

bnomei commented 4 years ago

this is strange. i expected either the kirbytext() to fail with an error but not to render partial output. can you PM me the relevant parts of your project in slack? i see no other efficient way to debug this otherwise.

colin-johnston commented 4 years ago

I agree at this stage a better view into the whole project will work best. I can cut a clean branch and share that with you on GitHub as soon as I’m at my desk again. I need to finish cleaning up some old JavaScript so it’s not in the way. Thanks again On Mar 7, 2020, 1:47 AM -0800, Bruno Meilick notifications@github.com, wrote:

this is strange. i expected either the kirbytext() to fail with an error but not to render partial output. can you PM me the relevant parts of your project in slack? i see no other efficiant way to debug this otherwise. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

colin-johnston commented 4 years ago

I was considering your suggestion to share the source of my project, and it inspired me to just set a clear baseline. I did a clean install of the latest kirby3-starterkit, installed the plugin, set up my test image and everything worked—which is expected, of course. Both share MAMP PRO PHP 7.2.1 so I rule that out. So I then went into my project and commented out all javascript src in the head, and only directly included lazysizes (5.2.0). Still the same issues as above. So poking around and looking for differences between starterkit and my project, and the only thing I can see is my custom setup for serving out of custom folders (see below). Since lots of the JS I have in here is old and fragile (seriously), I thought I'd remove it and hope it was the culprit—but I cleared my cache and saw the same behavior as before. Anyway, I thought I'd share the custom config and see if it looks like something that might cause an issue. I can't see any other differences so pretty stuck. I'll share a link in a Slack DM to a repo with copy of my project for you to look at. I hope it's an obvious issue coming from something I just didn't see.

<?php

include __DIR__ . '/../vendor/autoload.php';

$kirby = new Kirby([
    'roots' => [
        'index'    => __DIR__,
        'base'     => $base    = dirname(__DIR__),
        'content'  => $base . '/content',
        'site'     => $base . '/site',
        'storage'  => $storage = $base . '/storage',
        'accounts' => $storage . '/accounts',
        'cache'    => $storage . '/cache',
        'sessions' => $storage . '/sessions',
    ]
]);

echo $kirby->render();

(...this is served from /public which is the Apache root for site.)

colin-johnston commented 4 years ago

@bnomei 3.3.3 initially didn't work for me. I tracked it down to Markdown Extra.

Scenario 1 Disable Markdown Extra -> everything works including ratio inline class etc., but losing my additional markdown classes.

Scenario 2 Enable Markdown Extra and add ratio: false for each image - > everything but ratio works, and I have my added markdown classes.

But so much further along than I was—thanks for the update.

bnomei commented 4 years ago

ah. i had to create a custom driver. i need to take care of the extra option it seems.

colin-johnston commented 4 years ago

Looking at your new component addition is how I focused on the markdown settings. Very clever solution you came up with there—learned something here. Hope this all helps you too. On Mar 10, 2020, 3:12 AM -0700, Bruno Meilick notifications@github.com, wrote:

Reopened #21. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

bezin commented 4 years ago

Hello, I get an Reference Error in the panel, when this plugin is installed but the editor plugin is not.

ReferenceError: editor is not defined

This is caused by this: https://github.com/bnomei/kirby3-srcset/blob/master/src/index.js#L3

bnomei commented 4 years ago

@bezin can you check if if(editor) { editor.block("srcset", Srcset); } does solve it?

bezin commented 4 years ago

It does not, but my PR fixes it.

colin-johnston commented 4 years ago

@bnomei If you have time, could you point me in the direction of how to update the added component code to work with MarkdownExtra? I can't from the code here understand the specifics, but willing to learn if I can help. https://github.com/bnomei/kirby3-srcset/blob/dd6c6f322af1d1b2fe688e7ff2f090e85178b5a5/index.php#L60 Thanks!

bnomei commented 4 years ago

@colin-johnston found a fix. see v3.3.5