Now I am turning my attention to html and css. Not much, if any, of this will be errors. Much of it is probably MY personal opinion/preference. Please do not feel the need to change something if you prefer it the way it is.
My general reaction is that the html is bloated more than it needs to be. The same can be said for the css - at over 500 lines, it is bigger than some template's main stylesheet.

And this 500+ lines of css loads for every pageview, not just the event calendar.
Here is an example with error messages:
Code: Select all
<div class="serendipity_center eventcal_tpl_error">
<div class="eventcal_tpl_error_inner">{$plugin_eventcal_error}</div>
</div>
Which comes with this css:
Code: Select all
.eventcal_tpl_error {
border: 1px solid #6280A2;
background-color: #A29D8C;
color: #906030;
font-weight: 580;/**/
padding: 6px;
}
.eventcal_tpl_error_inner {
width: auto;
border: 1px solid #6280A2;
background-color: #FF9030;
padding: 6px;
}
Why two divs? Why all the css? Most templates have the class serendipity_msg_important, how about (with, or without the additional class of serendipity_center):
Code: Select all
<div class="serendipity_center serendipity_msg_important">{$plugin_eventcal_error}</div>
One less div, no plugin css at all.
Next, debug code and css. This is just from plugin_eventcal_cal.tpl:
Code: Select all
{if $is_eventcal_cal_debug_fdc}
<div id="eventcal_error_surrounder">
<div class="error_brand"> function: {$is_eventcal_cal_debug_fdc} </div>
</div>
{/if}
{if $is_eventcal_cal_debug_fcwe}
<div id="eventcal_error_surrounder">
<div class="error_brand"> function: {$is_eventcal_cal_debug_fcwe} </div>
</div>
{/if}
{if $is_eventcal_cal_debug_fs1}
<div id="eventcal_error_surrounder">
<div class="error_brand"> function: {$is_eventcal_cal_debug_fs1} </div>
</div>
{/if}
{if $is_eventcal_cal_debug_fs2}
<div id="eventcal_error_surrounder">
<div class="error_brand"> function: {$is_eventcal_cal_debug_fs2} </div>
</div>
{/if}
And the css that goes with it:
Code: Select all
#eventcal_error_surrounder {
padding: 4px;
background-color:#999;
border: 1px solid #333;
}
.error_brand {
background-color: #000;
color: #FF3000;
font-weight: 800;
}
Are all those debug conditions necessary? Do they really add value? 4 separate variables? As above, also has 2 divs where one do fine. My suggestion would be to again use the serendipity_msg_important class, and let the template provide css.
There are other error and/or debug conditions in other tpls. My preference would be to change all of them to use the existing serendipity_msg_important class as well, and only keep the debug variables and code that is absolutely necessary. Personally, when I debug something, I keep the code only as long as is necessary for debugging, then I remove it. Again, this is my personal preference - change only if you want to.
These error related classes are defined in the stylesheet, but do not appear in any plugin file:
Code: Select all
.error_table_main {
color: #333;
background-color: #F00000;
border: 2px solid #F0F0F0;
}
.eventcal_error_bundled {
width: auto;
border: 2px dashed #FF3300;
-moz-border-radius: .0em 0 1em 1em;
}
.eventcal_errors {
font-family: verdana,arial,helvetica,sans-serif;
font-size: 11px;
font-weight: 500;
text-align: left;
color: #333;
}
Regarding this:
Code: Select all
<div class="serendipity_center eventcal_tpl_message">
<div class="serendipity_center serendipity_msg_notice">{$plugin_eventcal_cal_admin}</div>
{foreach from=$plugin_eventcal_message item=message}
<div class="eventcal_tpl_message_inner">{$message}</div>
{/foreach}
</div>
which uses this css:
Code: Select all
.eventcal_tpl_message {
border: 1px solid #6280A2;
background-color: #C6C6DE;
padding: 6px;
}
.eventcal_tpl_message_inner {
width: auto;
border: 1px solid #6280A2;
background-color: #A29D8C;
color: #F5DEB3; /* Wheat */
padding: 6px;
}
I do like that serendipity_msg_notice was used, but I personally think the other classes are too much.