serendipity_plugin_entrylinks enhancements

[Alexander Heß]

I found some quirks in serendipity_plugin_entrylinks which should be fixed:

1. Add this code between lines 104 and 106:

Code: Select all

if ($serendipity['view'] != 'entry') {
    return false;
}

Reason:
Since this only works when viewing an entry it should only be displayed there. An empty headline looks really stupid.

2. Change the SQL statement in line 144 to this:

Code: Select all

SELECT link, max(name) as name FROM {$serendipity['dbPrefix']}references WHERE entry_id = " . $id . " AND type = '' GROUP BY link ORDER BY id

Reason:
The links are numbered in the database in the exact same order they appear in the entry. It's more logical if they get output in this order.

3. Remove all inline styles from the output (lines 146, 153, 172, 182).

Reason:
What’s the point of having external stylesheets when there are inline styles which have a higher priority? If I’m understanding right, it’s possible to add to the default stylesheet using a hook. This should be done to get the alignment right, since this can be overwritten using a later stylesheet.

[garvinhicking]

Hi!

1. In which case does that happen? Usually it should bail out in line 108 already, when $serendipity['GET']['id'] is not set.

2. Committed, thanks a lot!

3. The reason is backwards compatibility, to make that plugin work in all and older templates. You can override them in your CSS code, if you want to. Removing it would mean that it shows wrong in existing, older blogs when they update.

Best regards,
Garvin

[Alexander Heß]

Servus Garvin!

1. This happens on pages that come from serendipity_event_staticpage.

2. Cool, np.

3. I understand the compatibility reason but isn’t that what the addToCss method in the plugin class is for? The only possibility to override inline styles is using an !important rule in the external css which I don’t like at all. Most of the inline styles are to make something look “prettier” but don’t do any real layout work.

Cheers,

Alex

[garvinhicking]

Hi!

1. I don'T see anywhere in the staticpage PHP code where $serendipity['GET']['id'] gets set...are you sure of that?

3. I understand the compatibility reason but isn’t that what the addToCss method in the plugin class is for? The only possibility to override inline styles is using an !important rule in the external css which I don’t like at all. Most of the inline styles are to make something look “prettier” but don’t do any real layout work.

Sidebar plugins cannot use event hooks! So it cannot be emitted in the CSS, unless you install a second, paired event plugin...

I understand that you dislike !important, but that's what it's there for.

Regards,
Garvin

[Alexander Heß]

1. Yes, absolutely. But I actually have an idea what might be causing it:
My permalink scheme for articles is %title%.html – just like the static pages (and the contactform too), which are like /impressum.html.

Since var_dump($serendipity['GET']['id']) gives int(0) as result I had a look at the code that actually looks up the entry id by the permalink in index.php and I think line 281

Code: Select all

    $id = (int)$matches[1];

could be replaced with something like

Code: Select all

    if (intval($matches[1]) > 0) {
        $id = (int)$matches[1];
    } else {
        $id = false;
    }

Damn, I totally missed the fact that addToCss is exclusive to event plugins. How embarrassing…

I think the most elegant and flexible way to solve the “problem” with the inline styles would be to give those plugins that output html with inline styles a smarty template. I’d even volunteer to add that code to the entrylinks plugin (and maybe one or two more) if there’s a chance that this will be commited.

[garvinhicking]

Hi!

Ah, good catch! Thanks, I committed that $id fix. I always used %id% in the URL for performance reasons, so I didnT catch that.

I think the most elegant and flexible way to solve the “problem” with the inline styles would be to give those plugins that output html with inline styles a smarty template. I’d even volunteer to add that code to the entrylinks plugin (and maybe one or two more) if there’s a chance that this will be commited.

Performance wise that's a real killer, to employ a smarty template and render it for smaller plugins. Entrylinks is so terribly small, that adding the smarty overhead to it would be a bit uncomfortable.

But if you'd modify it so that it can be enabled optionally (just like with the recent comments and calendar plugin), I'd commit it.



Regards,
Garvin

[Matthias2]

I just installed this plugin, great!

Is it possible to deacivate this plugin for several articles?

[garvinhicking]

Hi!

I just installed this plugin, great!

Is it possible to deacivate this plugin for several articles?

No, currently it's on a global scope, since it's not regarded as a markup plugin. You could work around it with some smarty templating, but that would be rather complex and would require smarty templating knowhow...

Regards,
Garvin