(I'm marking this 'critical' because it's a beta breaker.)
At the beginning I followed Views' example and used a flag__ pattern for the template. Later I changed it to flag_ , because I thought users would feel easier with one dash, and because CCK too uses one dash.
But I think this will cause a problem in the future: if we'll have some template that starts with "flag_", but which doesn't theme a flag link, Drupal will consider it as such nevertheless.
CCK doens't have this problem, because it doens't use a pattern. It uses suggestions[].
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | flag_theme5.patch | 1.16 KB | quicksketch |
| #9 | flag_theme6.patch | 1.17 KB | quicksketch |
| #7 | flag_theme_dashes.patch | 631 bytes | quicksketch |
Comments
Comment #1
quicksketchI don't think this is necessarily critical, since right now we only have one kind of template available. However, I see your point regarding name-spacing in the future.
current: flag-bookmark.tpl.php
future(?):
flag--bookmark.tpl.php
flag--bookmark--link.tpl.php
flag--bookmark--[something else].tpl.php
Comment #2
quicksketchAh, after re-reading, I see where else we might have troubles. Say we created a "flag preview" that had a theme template of "flag-preview.tpl.php", that'd cause some serious problems if a user created a flag called "preview". An alternative to the double-dash approach though, could be to simply rename our template flag-link.tpl.php and flag-link-[bookmark].tpl.php.
Comment #3
mooffie commentedThe problem is as follows:
Now, suppose there's a "flag_" pattern, for the "flag" hook. Drupal, effectively, does "ls flag-*" and "flag-preview.tpl.php" is picked up and considered as belonging to this "flag" hook. Whereas it actually belongs to the "flag_preview" hook.
(This only happens if "flag-preview.tpl.php" is placed in the theme folder, because Drupal doesn't scan for files in the module folder.)
Comment #4
mooffie commentedThat's right. I was aware of this.
Nate, what do you think is nicer: having 'flag--bookmarks.tpl.php', or 'flag-link-bookmarks.tpl.php' ?
Comment #5
quicksketchI'd probably like "flag-link-bookmarks" more, as if we make another template, it might be something like "flag-confirm-bookmarks" or "flag-form-bookmarks". It makes our templates consistent in the future, rather than having one "flag--bookmarks" and other templates like "flag-confirm--bookmarks".
Comment #6
mooffie commentedI think we should wait a bit, till we see how we finally implement the confirmation-link feature.
I'm demoting this to 'normal'.
Comment #7
quicksketchAfter having some time to think about this, I've finally decided to just go directly for the two dashes approach. I'm much more comfortable with the two dashes (as probably are other users) now that D6 has been out for a while. Since I'd much rather change the API now than after our 1.0 version, I figured it's time to commit the change.
I also updated our handbook page at http://drupal.org/node/295373. I couldn't find any other pages mentioning the template files, so I think we're good on documentation.
Not applied to the D5 version, since it doesn't have pattern functionality.
Comment #8
mooffie commented1. flag_flag::theme_suggestions() needs updating too (just replace the '_' with '__').
2. 'theme/RAEADME.txt' too needs updating.
Note that this feature works in both D6 and D5, so make sure to update both branches. It's true that for D6 we utilize the theming system's Pattern mechanism, but for D5 we use the good 'ol "suggestions" mechanism (see flag_phptemplate_adapter()).
Comment #9
quicksketchThanks again mooffie!
Comment #10
mooffie commentedGreat. And thanks for resolving all these issues!