Here's a patch that separates the listing of events into an overridable theme function.

I've also added a theme_item_list at the end (with the class "gcal_events"), which makes the default output much more themable.

CommentFileSizeAuthor
#5 gcal_filter_access.patch856 bytesmsonnabaum
#5 gcal_css.patch1.71 KBmsonnabaum
gcal.patch2.21 KBmsonnabaum

Comments

Se7enLC’s picture

Se7enLC commented

I like the sounds of that!

I just applied the patch to my local copy, and I don't see any output. Does something else need to be done to enable it or is a newer version of drupal required?

Se7enLC’s picture

Se7enLC commented

disabling/re-enabling the module was required to make it show up.

From the looks of it, the theme_gcal_events_contents() function does not properly handle multiple blocks (the $which_block variable). Can that be added in?

I'm also seeing list bullets in front of blank lines and in front of the tables - is that intentional? It looks strange

So far it looks like it has some potential.

Se7enLC’s picture

Se7enLC commented

Solved the block number problem (had to add $which_block to the theme function)
The weird issue with tables was a non-standard configuration for a calendar block that I fixed.

The only remaining issue I have with using this patch is actually theming the block. I've added a custom css file, but I'm having issues actually overriding the styles of the block. Do you have an example css theme that you used? I was attempting to remove the list bullet display with "list-style: none;", but I can't get the right combination of nested classes to get it to take effect.

Se7enLC’s picture

Se7enLC commented

Apart from some minor issues with the theme css overriding the custom css settings for the block, the theme patch looks good. I just applied it to dev with a few small changes:
- added in the per-block support for footer and "no events" text
- added a checkbox into the admin panel to disable the new themes and go back to the old style

msonnabaum’s picture

msonnabaum commented
StatusFileSize
new gcal_css.patch1.71 KB
new gcal_filter_access.patch856 bytes

Changes seem to work fine for me with one exception.

This bit introduces some unexpected behavior:

    $input_format = variable_get('gcal_events_input_format', GCAL_EVENTS_DEFAULT_INPUT_FORMAT);
    if (!filter_access($input_format)) {
      drupal_set_message('Input Format ' . $input_format . ' is not accessible by the current user');
      return "Error: Input format is not accessible";
    }

It's unnecessary to check if the logged in user has access to the input format, because that user is simply viewing the content, not submitting content. This is why we need the "FALSE" argument when check_markup() is called, so that it skips filter_access(). Since check_markup is right below this, it seems rather redundant anyhow. Taking it out completely seems to be best option here.

Also for the css, I believe this should do what you want:

.item-list ul.gcal_events li {
  list-style: none;
  margin-left: 0px;
  padding-left: 0px;
}

For those of us who may want to bullets and padding, this is much simpler to override than what's currently there.

I also saw a bunch more entries with different class names (i.e. gcal_events_tomorrow), but I didn't see anywhere in the module where these classes would be used, so for now I took them out. If these classes do showup however, I'd suggest adding them in addition to the "gcal_events" class, so that you don't have to have a bunch of redundant CSS.

Patches for both attached!

Se7enLC’s picture

Se7enLC commented

That first section was something I added in to generate an error message when an input filter is not available (since normally it would just error out and cause a blank page). If changing that option to false will fix that, I like that solution better. I'll try out that patch.

You would not BELIEVE the number of iterations of CSS blocks I went through to find something that even worked. There are some issues with the Sky theme that prevented the normal override from working. That segment seems to work now, but I believe I made some changes to the default styles.css file for the theme to enable it.

The _tomorrow, _today, _thisweek, _thismonth, and _other are used when you enable "day grouping' in the main module settings. That groups the events into sections with headings. To prevent the headings themselves from becoming LI tags, I had to split up into multiple blocks. I could have named them all gcal_events, but I figured naming them differently would be more flexible. I could probably make that an option. Do you know if the theme() function will allow multiple class definitions, or would I need to put the entire section into a div tag?

Se7enLC’s picture

Se7enLC commented

Try your css changes with the "Sky" module - I don't think they work.

jdwfly’s picture

jdwfly commented

This either needs to be committed or scrapped.

jdwfly’s picture

jdwfly commented
Issue summary: View changes
Status: Needs review » Closed (fixed)

There is a theme function in the latest releases.