getgrav / grav-plugin-shortcode-owl-carousel

Grav Shortcode Owl Carousel Plugin
https://getgrav.org
MIT License
11 stars 12 forks source link

external css order not consistent #8

Open olivierdalang opened 7 years ago

olivierdalang commented 7 years ago

Hi,

When I pushed my website online, owl externals css where not loaded in the same order than on my local developpement server.

Local :

<link href="/test/user/plugins/shortcode-owl-carousel/css/owl.carousel.min.css" type="text/css" rel="stylesheet" />
<link href="/test/user/plugins/shortcode-owl-carousel/css/owl.theme.default.min.css" type="text/css" rel="stylesheet" />
<link href="/test/user/plugins/shortcode-owl-carousel/css/shortcode.owl.carousel.css" type="text/css" rel="stylesheet" />

Online :

...
<link href="/test/user/plugins/shortcode-owl-carousel/css/shortcode.owl.carousel.css" type="text/css" rel="stylesheet" />
<link href="/test/user/plugins/shortcode-owl-carousel/css/owl.theme.default.min.css" type="text/css" rel="stylesheet" />
<link href="/test/user/plugins/shortcode-owl-carousel/css/owl.carousel.min.css" type="text/css" rel="stylesheet" />
...

This leads to different rendering, especially the left-right arrows that have an ugly gray background instead of the transparent black one.

I don't know if the bug is in grav, shortcode-code or this repo though ?

olivierdalang commented 7 years ago

I can fix the problem by changing (in OwlCarouselShortcode.php)

...
$this->shortcode->addAssets('css', 'plugin://shortcode-owl-carousel/css/owl.carousel.min.css');
$this->shortcode->addAssets('css', 'plugin://shortcode-owl-carousel/css/owl.theme.default.min.css');
// load animate.css if required
if ($sc->getParameter('animate') == 'true') {
    $this->shortcode->addAssets('css', 'plugin://shortcode-owl-carousel/css/animate.css');
}
// load built-in-css
if ($this->config->get('plugins.shortcode-owl-carousel.built_in_css', false)) {
    $this->shortcode->addAssets('css', 'plugin://shortcode-owl-carousel/css/shortcode.owl.carousel.css');
}
...

to

...
$this->shortcode->addAssets('css', ['plugin://shortcode-owl-carousel/css/owl.carousel.min.css',1]);
$this->shortcode->addAssets('css', ['plugin://shortcode-owl-carousel/css/owl.theme.default.min.css',4]);
// load animate.css if required
 if ($sc->getParameter('animate') == 'true') {
    $this->shortcode->addAssets('css', ['plugin://shortcode-owl-carousel/css/animate.css',3]);
}
// load built-in-css
if ($this->config->get('plugins.shortcode-owl-carousel.built_in_css', false)) {
    $this->shortcode->addAssets('css', ['plugin://shortcode-owl-carousel/css/shortcode.owl.carousel.css',2]);
}
...

I still don't get how it could be that they loaded in the right order on my local server, and why this behaves differently on local and distant server ?

rhukster commented 7 years ago

Why not submit a PR for this?

mahagr commented 7 years ago

@rhukster It sounds like a bug to me as the assets should keep the order they were added in.

rhukster commented 7 years ago

Ok, looking at this, it seems they are added by default with an order based on a md5 of the asset (to ensure no duplicates are added), this md5 must be different on different machines (probably due to path differences) and that is throwing the order. I'll dig deeper.

rhukster commented 7 years ago

I've looked at this in-depth and asset sorting is working 100% as it should as far as I can tell. Basically there's two factors in sorting, first the order in which the assets are added. Up to a couple of versions ago (of Grav core), plugins were processed in the order they were found in the filesystem, now they are sorted based on their name, so it's more consistent. Inside a plugin, the order should be the order they are added via the ->addCss type methods.

This 'order' is used for sorting unless there is a 'priority' field provided, if provided that is taken into account, else the 'priority' is 10 and then the order value will dictate the sort order.

I can't see how this is getting out of order on different systems, as the md5 hash is not actually used for sorting at all, it's purely for detecting duplicates.

mahagr commented 7 years ago

I checked the code and I agree. The only reason why the ordering of the assets would be different is that something else loads some of the assets first -- or before your code from above.

miraculli commented 6 years ago

I´ve the same problem here. fixed with settinge the prio to the external css (shortcode.owl.carousel.css) to 5 (below 10) like described in this note

Also it seems to be difference if the plugin is used more than once on the same page.