Hi

The clocks didn't look right in a content block displayed vertically as it left too much space around the block and used up a lot of vertical space. I have created a patch to display the clocks block horizontally, i added an option into the configuration options for the clocks. I have 4 clocks displayed in this format in a content block.

I have had to add in some alignment to the <td> tags which also align the clocks in vertical mode but i think it looks quite good that way too.

I've been coding in php for ages but this is my first drupal contribution so please let me know what you think.

Cheers

Comments

munkiepus’s picture

Just wondered if i should have created the patch for the 6.x-1.x-dev version instead of the beta1 version, let me know and i'll make the patch in the dev version instead.

Cheers

flevour’s picture

You usually roll patches against the dev version, but in this case there shouldn't be many differences between the 2 versions.
I'll try to have a look at your patch today. Can you link to an example of the horizontal patch in action? Screenshot are also as good.
Have a nice day,
Francesco

munkiepus’s picture

StatusFileSize
new12.99 KB

Here is a working example of the worldclock in horizontal configuration http://www.munkiepus.com/worldclock,

i tested the patch on the 6.x-1.x-dev and it works fine, the site i have above is using a patched 6.x-1.x-dev version of the worldclock module.

I have attached a screenshot of the configuration page with the added option.

munkiepus’s picture

StatusFileSize
new1.64 KB
new2.09 KB

Here are 2 screenshots of the clock in horizontal display in a content block.

These are from 2 different sites with different themes.

1st attachment has 3 clocks in a 3 column theme

2nd attachment has 4 clocks in a 2 column theme

flevour’s picture

Version: 6.x-1.4-beta1 » 6.x-2.x-dev

I branched for the v2 today, would you please re-roll against DRUPAL-6--2 please? I suggest getting the code out of CVS so we can work together better. See how to create a patch and Checking out from the contributions repository inside the CVS handbook. Thanks for your interest in contributing!

munkiepus’s picture

StatusFileSize
new2.64 KB

Hi,

I've downloaded and re created the patch against the most recent dev version 6.x-2.x-dev,

I got the code from the cvs and applied the attached patch to the module with no issues.

Cheers, Tony

flevour’s picture

Hi Tony,
today I'll have a look at the patch. To checkout the current v2 code from CVS you need to add the "-r DRUPAL-6--2" part

munkiepus’s picture

Yeah i figured the "-r DRUPAL-6--2" part out after my original post, got it now :)

munkiepus’s picture

Status: Active » Needs review

Not sure if you liked this or not, anything i could do to improve it?

I was thinking that a rows and columns setup might be useful if you wanted to display more than a single row or column.

Any thoughts?

flevour’s picture

Hi Tony,
I was sure I shared some thoughts with you about this, but it turns out they were stuck in my mind.

It is doubtless that the current theming implementation has a lot of limits. The major one in my opinion is that there is too much logic embedded in the theme function.
In my experience it is also very uncommon that a module provides theming options as this one. Nevertheless, people are expect such features from Worldclock, so it is not advisable to take them away.

There are a couple of ideas worth exploring:
1) perusing the existing theme_table function provided by Drupal;
2) moving HTML code to a tpl.php template and writing a preprocess function.

In the second case, the preprocess function could provide a variety of useful variables. The module would also provide a default clocks-list.tpl.php and could also contain a subfolder with drop-in replacements of clocks-list.tpl.php, each implementing a different layout. In this scenario, a revised version of your code would be in this subfolder.

What do you think?
Ideas anyone?
Happy 2009!
Francesco

munkiepus’s picture

Just to let you know, i'm working on the tpl.php and preprocess function now.

Should have something soon.

munkiepus’s picture

Priority: Minor » Normal
Status: Needs review » Active

Thought i'd better change the status of this issue too.

flevour’s picture

Great! Thanks for your active contribution, I'm a bit busy in this period and still have to find out a way to allocate time to this project again!
Talk soon,
Francesco