major upgrade lightbox and usergallery plugins

Creating and modifying plugins.
User avatar
yellowled
Regular
Posts: 7002
Joined: Fri Jan 13, 2006 12:46 pm
Location: Eutin, Germany
Contact:

Re: major upgrade lightbox and usergallery plugins

Post by yellowled » Wed Feb 03, 2016 12:55 pm

yellowled wrote:And if those work as well, I believe we can drop the fixchrome.css altogether?
Well, we'd need to add said CSS to s9y's fallback styles, I guess. Which would make this another case of “only release the plugin update after an updated version of s9y has been released”, which is kind of a bummer. But I guess we can do that.

YL
amazon Wishlist - Serendipity-Podcast (German only, sorry)

User avatar
yellowled
Regular
Posts: 7002
Joined: Fri Jan 13, 2006 12:46 pm
Location: Eutin, Germany
Contact:

Re: major upgrade lightbox and usergallery plugins

Post by yellowled » Wed Feb 03, 2016 5:31 pm

yellowled wrote:I'll leave this open for other people to confirm, and then I'll test it with the other lightbox scripts as well. And if those work as well, I believe we can drop the fixchrome.css altogether?
For the record, I just quickly tested this with prettyPhoto, ColorBox and ven jQuery Lightbox 2 – it does solve the issue for all of them.

YL
amazon Wishlist - Serendipity-Podcast (German only, sorry)

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: major upgrade lightbox and usergallery plugins

Post by Timbalu » Wed Feb 03, 2016 5:57 pm

Great! Was this a matter of an updated webkit? Or does it even work with "older" webkit engined Browsers (lets say ~1 year old...)?
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian

User avatar
yellowled
Regular
Posts: 7002
Joined: Fri Jan 13, 2006 12:46 pm
Location: Eutin, Germany
Contact:

Re: major upgrade lightbox and usergallery plugins

Post by yellowled » Wed Feb 03, 2016 10:53 pm

Timbalu wrote:Was this a matter of an updated webkit? Or does it even work with "older" webkit engined Browsers (lets say ~1 year old...)?
I have no older WebKit browser available, so I could not even test it.

It's not as if the issue is fixed just by an updated WebKit (or lightbox plugin, for that matter) – it still needs a CSS fix (in form of display: block on .serendipity_image_link), and while that is kind of similar to your previous fix, I assume this to be way less prone to error in relation to other (theme) styles just because of the fact that it does not generate a :before pseudo element or require a @media query.

So if you're asking “Will this work in older versions of Chrome or Safari?“ – I don't know. I don't consider it to be a problem in Chrome since it's an evergreen browser. Unfortunately, Safari is not. But since there is no good way to test that and no good way to only use a different fix for older Safaris (if that were necessary, which we don't know), I'd say we apply the new fix and wait for people to report errors (which I don't anticipate ever happening).

I would like to proceed as follows:
  • add the fix to the lightbox plugin instead of the old fix in fixchrome.css for the time being
  • add it to master in /templates/default/style_fallback.css so that we can remove fixchrome.css from the plugin once 2.1 is released
Does that seem reasonable?

YL
amazon Wishlist - Serendipity-Podcast (German only, sorry)

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: major upgrade lightbox and usergallery plugins

Post by Timbalu » Thu Feb 04, 2016 11:24 am

yellowled wrote:add it to master in /templates/default/style_fallback.css so that we can remove fixchrome.css from the plugin once 2.1 is released
No. We can't do this for compat. But we can write a condition for >= 2.1 to not use it.
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian

User avatar
yellowled
Regular
Posts: 7002
Joined: Fri Jan 13, 2006 12:46 pm
Location: Eutin, Germany
Contact:

Re: major upgrade lightbox and usergallery plugins

Post by yellowled » Thu Feb 04, 2016 11:45 am

Personally, I think the best solution for this would be addToCSS – emit the fix if it's not in your theme's stylesheet, no matter which s9y version.

I'm not sure I can figure out how to implement that, but I'll try.

YL
amazon Wishlist - Serendipity-Podcast (German only, sorry)

User avatar
yellowled
Regular
Posts: 7002
Joined: Fri Jan 13, 2006 12:46 pm
Location: Eutin, Germany
Contact:

Re: major upgrade lightbox and usergallery plugins

Post by yellowled » Thu Feb 04, 2016 11:59 am

yellowled wrote:Personally, I think the best solution for this would be addToCSS
After looking at how that works, nope. addToCSS just checks if the class in question is defined in the CSS at all, it does not check if a certain property has as certain value. Too prone to error.

So, how do we do this switch?

Code: Select all

if ($serendipity['version'][0] == '1') {
    echo '    <link rel="stylesheet" type="text/css" href="' . $pluginDir . '/fixchrome.css" />' . "\n";
} else {
    if ($serendipity['version'][1] < '1') {
        echo '    <link rel="stylesheet" type="text/css" href="' . $pluginDir . '/fixchrome.css" />' . "\n";
    }
}
That seems incredibly clumsy. There's probably a better way to do this. I'll leave it to you.

YL
amazon Wishlist - Serendipity-Podcast (German only, sorry)

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: major upgrade lightbox and usergallery plugins

Post by Timbalu » Thu Feb 04, 2016 3:25 pm

Code: Select all

if (strpos($eventData, '.serendipity_BlahBlah') === false) {
    $this->addToCSS($eventData);
}
or

Code: Select all

if (strpos($eventData, '.serendipity_BlahBlah') === false) {
    $eventData .= 'fake example for the styles you wish to append';
}
I normally do for 2.1 cases

Code: Select all

if (version_compare($serendipity['version'], '2.0.99', '<')) {
    $eventData .= 'fake example for the styles you wish to append';
}
or

Code: Select all

if (version_compare($serendipity['version'], '2.0.99', '<')) {
    echo '    <link rel="stylesheet" type="text/css" href="' . $pluginDir . '/fixchrome.css" />' . "\n";
}
But personally I try to avoid checks like that when possible, since that happens on each frontpage request - and each check, even when singulary fast on its own, sums down performance at the very end.
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian

User avatar
yellowled
Regular
Posts: 7002
Joined: Fri Jan 13, 2006 12:46 pm
Location: Eutin, Germany
Contact:

Re: major upgrade lightbox and usergallery plugins

Post by yellowled » Thu Feb 04, 2016 6:48 pm

Timbalu wrote:But personally I try to avoid checks like that when possible, since that happens on each frontpage request - and each check, even when singulary fast on its own, sums down performance at the very end.
I can not measure or judge that. All I know is that referencing an external stylesheet for something that boils down to a single line of CSS is insane.

I'll commit the 4th example (thanks) to get this over with, the performance stuff is a separate discussion anyway.

YL
amazon Wishlist - Serendipity-Podcast (German only, sorry)

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: major upgrade lightbox and usergallery plugins

Post by Timbalu » Thu Feb 04, 2016 7:15 pm

Ah - now I see what you were up for.
This is a single line - we should not add an extra file for this!
Can't we use that global for all browsers? And in the plugin only?
If you answer me this, I'll better add that before the sync.
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian

User avatar
yellowled
Regular
Posts: 7002
Joined: Fri Jan 13, 2006 12:46 pm
Location: Eutin, Germany
Contact:

Re: major upgrade lightbox and usergallery plugins

Post by yellowled » Thu Feb 04, 2016 7:24 pm

Timbalu wrote:Can't we use that global for all browsers?
I have never tested this in any IE or an Android browser, but I assume so. I also assume it does not do any harm if the plugin is not installed, it may even make image links more clickable (because the link now spans the whole image, not just the bottom). That's why I would leave it in style_fallback.css either way.
Timbalu wrote:If you answer me this, I'll better add that before the sync.
I assume you saw b6b0cc. If you can change that before the sync to use addToCSS, great. If not, no biggie (Personally, I only care if this works properly in > 2.1.)

YL
amazon Wishlist - Serendipity-Podcast (German only, sorry)

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: major upgrade lightbox and usergallery plugins

Post by Timbalu » Thu Feb 04, 2016 7:37 pm

Done fd32bc8. Can you please test this with FF and without the 2.1 fallback commit?
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian

User avatar
yellowled
Regular
Posts: 7002
Joined: Fri Jan 13, 2006 12:46 pm
Location: Eutin, Germany
Contact:

Re: major upgrade lightbox and usergallery plugins

Post by yellowled » Thu Feb 04, 2016 7:51 pm

Timbalu wrote:Can you please test this with FF and without the 2.1 fallback commit?
Sorry, I don't understand what you want me to test with what? :?

YL
amazon Wishlist - Serendipity-Podcast (German only, sorry)

User avatar
Timbalu
Regular
Posts: 4598
Joined: Sun May 02, 2004 3:04 pm

Re: major upgrade lightbox and usergallery plugins

Post by Timbalu » Thu Feb 04, 2016 7:53 pm

test https://github.com/s9y/additional_plugi ... 24db739eb5 without the 2.1 fallback commit ...
Regards,
Ian

Serendipity Styx Edition and additional_plugins @ https://ophian.github.io/ @ https://github.com/ophian

User avatar
yellowled
Regular
Posts: 7002
Joined: Fri Jan 13, 2006 12:46 pm
Location: Eutin, Germany
Contact:

Re: major upgrade lightbox and usergallery plugins

Post by yellowled » Thu Feb 04, 2016 7:59 pm

If the fix is in /templates/default/style_fallback.css, it is still emitted in serendipity.css by the lightbox plugin. I assume that is what you meant. If it is missing in style_fallback.css, it is still emitted by the plugin in serendipity.css.

YL
amazon Wishlist - Serendipity-Podcast (German only, sorry)

Post Reply