More themeability
stBorchert - May 30, 2009 - 12:10
| Project: | Mollom |
| Version: | 6.x-1.x-dev |
| Component: | User interface |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
Hey.
I've tried to theme the "play audio CAPTCHA" link and put the image (and the link) into a custom HTML structure but without any theme functions there is no chance to do this.
So here you are: 4 tiny theme functions to output the links and the image.
Stefan
| Attachment | Size |
|---|---|
| mollom_more-themeability.patch | 3.44 KB |
#1
Seems like it would be a good improvement. The only problem I would have is that in theme_mollom_captcha(), the
<div id="captcha">is required for the JavaScript switching to work. To me having this entire thing themed implies that it isn't a required, and this could cause problems if it is not properly override without the div.#2
1. If the ID is required, we could write
#prefix => '<div id="captcha">' . theme('mollom_captcha', $response) .'</div>',or would that be blasphemy for designers? Still sounds better than hard-coding everything so certainly a step forward.2. I'd remove theme_mollom_audio_captcha_link and theme_mollom_image_captcha_link and just go with their parent theme function: theme_mollom_captcha. Too many small theme functions can be a pain too, IMO.
3. The next step would be to make actual template files, of course, instead of theme functions. :-)
#3
#4
Hey.
I've reworked the patch and introduced 2 template files:
* mollom-audio-captcha.tpl.php
* mollom-image-captcha.tpl.php
Stefan
#5
It would be great to get these re-rolled against the latest version :-)
pretty please?
Jen
#6
I've tested it right now and it still applies without any problems to latest HEAD.
No need to reroll, only "committed to head" is missing :-)
#7
Do we *really* need template files for a link? Isn't using theme() enough to be able to override how it is displayed?
#8
Just theming the link title