Updating additional_plugins

[yellowled]

As you all know, I'm committed to contribute to s9y further by not only editing the emitted HTML code for core plugins, but also the so-called additional_plugins. Now, before I begin, I wanted to check on some ground rules with you guys.

A. If I actually change the emitted code, am I supposed to incement the version number of the plugin in question? Is it maybe even necessary for the new version to show up in Spartacus? Is there any common pattern for plugin version numbering?

B. A lot of the potential changes will be about removing inline styles. Inline styles really are a pain in the ass. However, it might be necessary to keep some base styles for the plugin in order to keep it usable. How do I go about this? Can I add a "plugin-specific stylesheet" which will be added to serendipity.css dynamically? I know some plugins do this, but how do I actually add something like that to a plugin?

YL

[garvinhicking]

Hi!

A. Incrementing the version number is always a good thing. The only reason not to do it is because of forgetting it, or being to stressed to remind oneself of incrementing it. So: Do it always.

The numbering is simply always incresed by ".1": 1.1, 1.2, 1.3, ... and then 1.10 and so forth. After 1.10 there's 1.11 or 1.20 but NOT 1.2 - because this is "smaller than 1.10".

Major version increments should only happen when important functionality changes.

B. is harder to answer. One needs to weigh the costs of breaking something working fine against the benefit removing inline styles brings. Breaking plugins just for the sake of pedantics is surely not the way to go. So it depends on the actual usage. Additional stylesheets are always a performance factor, so I cannot suggest a good way to deal with this. It would need a case-by-case ruling.

added to serendipity.css dynamically? I know some plugins do this, but how do I actually add something like that to a plugin?

This is not possible for sidebar plugins, only event plugins using the "css" event hook.

Regards,
Garvin

[yellowled]

A. Incrementing the version number is always a good thing.

So when I edited serendipity_event_staticpage to use <strong> instead of <b>, I should have edited

Code: Select all

$propbag->add('version', '3.74');

And set it to 3.75, correct?

Hm, I forgot about that then. Maybe I was too stressed

One needs to weigh the costs of breaking something working fine against the benefit removing inline styles brings.

"Working fine". That's a pretty relative thing. If a plugin's HTML code is virtually "unstyleable" because one has to overwrite every single thing using !important, that's cleary not "working fine".

Breaking plugins just for the sake of pedantics is surely not the way to go.

I'm not being pedantic here. A lot of the plugins use HTML and CSS from, well, the stone age.

I really don't mean to step on anybody's toes here. I am aware that semantic HTML and cross-browser CSS are not the expertise of a PHP coder. I know this stuff inside out, but I can't for the life of me grasp PHP enough to even code a plugin myself.

What I can do is offer my expertise to make these plugins better, and I firmly believe that using deprecated HTML with loads of inline styles is not the way to go. If anybody is opposed to that, just say it out loud, and I won't touch a thing. However, that would strongly limit the possibilities of s9y in the long run.

So it depends on the actual usage.

Of course it does

In a lot of cases, changing the emitted HMTL will already make most of the inline styles redundant. You can see that in my changes to the core plugins. That's one of the reasons why we should do this to begin with: using the semantically correct HMTL will make it far more easier to style the emitted code!

This is not possible for sidebar plugins, only event plugins using the "css" event hook.

Well, in that case, it's pretty pointless anyway. I'm going to have to use inline styles since the sidebar plugins are the ones most likely to be edited. Lots of event plugins don't even emit frontend HTML, so the focus here really is on sidebar plugins. That's fine, but I really suggest to keep inline styles to a minimum, if they're necessary at all.

Let's just see how this turns out once I get started. Maybe we're just discussing stuff here which will become obsolete after all (And after all, we can always reverse the changes thanks to cvs.)

YL

[garvinhicking]

Hi!

And set it to 3.75, correct?

That would've been the best way. But since its a minor change, bumping the version is not a matter of life and death *g*

"Working fine". That's a pretty relative thing. If a plugin's HTML code is virtually "unstyleable" because one has to overwrite every single thing using !important, that's cleary not "working fine".

Yes, but the other way would be: How would the output look like without the inline style? If it's a difference of display:block vs. display:inline, that could be a really huge impact. This has to be taken into consideration.

I really don't mean to step on anybody's toes here. I am aware that semantic HTML and cross-browser CSS are not the expertise of a PHP coder. I know this stuff inside out, but I can't for the life of me grasp PHP enough to even code a plugin myself.

No offense taken nor intended. But I must see these things a bit more pragmatic. If changes break things for worse, those must be thoroughly considered.

We have done that in the past, so as long as changes are virtually like the changes that we did before, I'm perfectly fine with them.

So, just keep your patches coming. If I am against some change (or someone else is), we can openly discuss it.

Regards,
Garvin

[yellowled]

Yes, but the other way would be: How would the output look like without the inline style? If it's a difference of display:block vs. display:inline, that could be a really huge impact. This has to be taken into consideration.

Yes, but in a lot of cases, this won't even be necessary. For instance, if an element uses "display: block;", and is substituted with a block level element (i.e., an element which has "display: block;" by default), the inline style becomes redundant but does not require the inline styling any longer. And in most cases, this will just be the "natural" way to change these things.

We have done that in the past, so as long as changes are virtually like the changes that we did before, I'm perfectly fine with them.

See, that's just what I'll be going for: make it look like before, but with better code underneath

YL

[yellowled]

We have done that in the past, so as long as changes are virtually like the changes that we did before, I'm perfectly fine with them.

Let me give you an example: I have just now edited my local copy of serendipity_plugin_authors to emit an actual (unordered) list. The old code emit <a> elements seperated by <br /> elements. So the new code looks like this:

Code: Select all

 echo '<ul>';

        foreach ($row_authors as $entry) {
            if (function_exists('serendipity_authorURL')) {
                $entryLink = serendipity_authorURL($entry);
            } else {
            	$entryLink = serendipity_rewriteURL(
                               PATH_AUTHORS . '/' .
                               serendipity_makePermalink(
                                 PERM_AUTHORS,
                                 array(
                                   'id'    => $entry['authorid'],
                                   'title' => $entry['realname']
                                 )
                               )
                             );
            }

            echo '<li><a href="' . $entryLink . '">' . $entry['realname'] . '</a></li>';
        }
        echo '</ul>';

This will in most templates look like a "normal" unordered list with bullets, padding, and margin. It will most likely need some extra CSS like at the end of /templates/default/style.css to look like it looked before.

I assume this is perfectly fine without inline styles, correct?

YL

[garvinhicking]

Hi!

How about updating all s9y styles to add a new class like "plain" that can be used for <ol> and <ul> lists to prevent bulletting and indenting?

Then plugins like this one can use the "plain" class. When all new templates support this, I think people can live with their old templates showing up with bullets and indents, because they can also easily implement the "plain" class?

Regards,
Garvin

[yellowled]

How about updating all s9y styles to add a new class like "plain" that can be used for <ol> and <ul> lists to prevent bulletting and indenting?

I'm not sure "plain" is a good choice for the class name (too generic for my taste), but the idea itself is of course a good one. Saves a lot of CSS, too

I'll come up with something and commit the changes later.

YL

[garvinhicking]

Hi!

Personally, I like short class names and often use the inheritcance feature of s9y to indicate proper nesting.

The benefit would be that you could use the plain class also for blog entries etc.

Which wording would you prefer?

Regards,
Garvin

[yellowled]

Personally, I like short class names and often use the inheritcance feature of s9y to indicate proper nesting.

The benefit would be that you could use the plain class also for blog entries etc.

I also like short names for classes and ids, and I often find myself mumbling rants about #serendipityLeftSideBar and stuff like that

I just figure class="plain" might be something people would use i.e. in templates to indicate other "undecorated" text element, which might lead to side effects.

I was thinking something like class="plain_list", actually. Or plainList if you prefer camel caps. Then again, plain_list doesn't really indicate what the class is actually doing. On the other hand, "list_without_bullets_or_indentation" is way too long

YL

[garvinhicking]

I like plainList!

[yellowled]

I like plainList!

I thought so

Just to avoid mistakes, and yes, I am aware of the fact that I ask this every single friggin' time, and I'm sorry about that: if I add this for core plugins, do I commit those changes to trunk only or also to branches/1.4/?

YL

[garvinhicking]

Hi!

I think committing it to both branch and trunk would be preferrable. We can put this into 1.4.2.

Regards,
Garvin