Greetings! I'm a fairly new drupal hacker and I must say that for the most part it's been a great experience working with drupal. However, I think hook_link() could really be improved. The problem: Any and all modules can add to $links, which is great. The issue I have is twofold: 1). The links that are added are generally HTML (presentation) rather than straight data. 2). All links are folded into a non-keyed array, so the themes have no idea which links came from which modules. It would be great if instead of this: $links[] = l(t('read more'), "node/$node->nid", array('title' => t('Read the rest of this posting.'), 'class' => 'read-more')); Modules were doing something like this: $links['mymodule'] = array('title' => t('Read the rest of this posting.'), 'url' => "node/$node->nid", 'class' => 'read-more'); And relying on the themes to actually display the link. I'd be happy to help code/patch this up, but I wanted to bounce the idea off of some of you first to get things rolling. It looks like I'm going to need something like this sooner rather than later, but I'd like to at least be hacking the core in such a way that my changes could eventually make it into a release, if this is deemed to be a good path to take. Regards, Justin aka {datura} on irc.
Justin See this http://drupal.org/node/636 And this http://drupal.org/node/18260 See if you can help by testing/improving.
I like this concept a lot. It bothers me to have 'add a comment' links on teasers. I don't anyone adding comments unless they've read the whole thing. I'd love to see any easy way to get rid of those. -Dave On Wednesday 10 May 2006 15:39, Just Justin wrote:
Greetings!
I'm a fairly new drupal hacker and I must say that for the most part it's been a great experience working with drupal.
However, I think hook_link() could really be improved.
+1 for the idea of separating the $links soup into several keys I'm not sure this is coherent with the rest of core, but I like the possibilites left to theme (filtering out, ordering...) I think leaving the render of the links to the themes is not a good idea, however : Having html code produced by l ("el"...) outside a theme function is considered OK, so what's the point of having a supplemental layer ? (plus you'd have to support the other params l handles : querystring, fragment...) Besides, modules might want to output other things than an actual <a> tag (simple text, whatever...). They are currently allowed to do so, which I think is a good thing. If their output is "complicated", it is up to the module coder ("good coding practice") to have a theme function provide that output.
Hi Yves, I think you have a good point regarding the rendering of the links. Perhaps it would be better to separate this into two issues: 1 - add keys to links 2 - change links to be rendered by themes or modules (I don't feel like I know enough about drupal to say what's right here) But, in my world it's really only #1 that is important. If it is a minor change, perhaps we could implement this first and deal with #2 after #1 is in place. Regards, Justin On Thu, May 11, 2006 at 01:23:55AM +0200, Yves CHEDEMOIS wrote:
+1 for the idea of separating the $links soup into several keys I'm not sure this is coherent with the rest of core, but I like the possibilites left to theme (filtering out, ordering...)
I think leaving the render of the links to the themes is not a good idea, however :
Having html code produced by l ("el"...) outside a theme function is considered OK, so what's the point of having a supplemental layer ? (plus you'd have to support the other params l handles : querystring, fragment...)
Besides, modules might want to output other things than an actual <a> tag (simple text, whatever...). They are currently allowed to do so, which I think is a good thing. If their output is "complicated", it is up to the module coder ("good coding practice") to have a theme function provide that output.
On 5/11/06, Just Justin <rocketfuel@spaceship.com> wrote:
It would be great if instead of this:
$links[] = l(t('read more'), "node/$node->nid", array('title' => t('Read the rest of this posting.'), 'class' => 'read-more'));
Modules were doing something like this:
$links['mymodule'] = array('title' => t('Read the rest of this posting.'), 'url' => "node/$node->nid", 'class' => 'read-more');
Smells like a FAPI-ified hook_link() to me. Mmmm... FAPI-ifyyyyy... +1. Cheers, Jaza.
It would be great if instead of this:
$links[] = l(t('read more'), "node/$node->nid", array('title' => t ('Read the rest of this posting.'), 'class' => 'read-more'));
It wouldn't be so bad if modules actually did this - but only some modules actually output a class on their links. Having some links with CSS and some without is a frustrating thing for a themer. I've filed at least 2 bugs on modules not including classes.
Modules were doing something like this:
$links['mymodule'] = array('title' => t('Read the rest of this posting.'), 'url' => "node/$node->nid", 'class' => 'read-more');
And relying on the themes to actually display the link.
In this situation, a theme could add a class to every link because it knows what module outputted the link. At a minimum, perhaps the code that concatenates all the links should automatically include the module's name as a class? -Mark
participants (6)
-
Dave Cohen -
Jeremy Epstein -
Just Justin -
Khalid B -
Mark Fredrickson -
Yves CHEDEMOIS