Posted by Digidog on August 30, 2011 at 12:42pm
3 followers
| Project: | Local tasks blocks |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | lotyrin |
| Status: | closed (works as designed) |
Issue Summary
so in line 105 and 107 of local_tasks_block.module
<?php
if ($tasks = render(menu_primary_local_tasks())) {
_local_tasks_blocks__context($delta, TRUE);
$content = "<ul class=\"primary\">\n" . $tasks . "</ul>\n"; // line 105
if ($tasks = render(menu_secondary_local_tasks())) {
$content .= "<ul class=\"secondary\">\n" . $tasks . "</ul>\n"; // line 107
}
?>it rather should be something like
<?php
if ($tasks = render(menu_primary_local_tasks())) {
_local_tasks_blocks__context($delta, TRUE);
$content = "<ul class=\"primary-block\">\n" . $tasks . "</ul>\n"; // line 105, added "-block"
if ($tasks = render(menu_secondary_local_tasks())) {
$content .= "<ul class=\"secondary-block\">\n" . $tasks . "</ul>\n"; // line 107, added "-block"
}
?>(and also in line 151 and 156 again) to have a default list-view instead of a site-breaking system-tabs view, because, mostly it will be a sidebar or a block which isn't wide enough to show the tabs like system default. By the way, it makes it possibly easier to define new css than to try overriding system css by the right css>selector>parameter overrides and makes it easier to switch between the tabs view of the module and the tabs view of the system if wanted.
Sorry that I have no time to provide a patch yet. But if maybe there is somebody to jump in? Otherwise I will make it later ...
Comments
#1
ok, here's the patch
#2
Thanks for posting this, I was trying to figure out why I was getting two horizontal tabs at the top of my menu block in v 6.x-1.x-dev, changing the class names fixed it.
#3
I don't really like this change, since it doesn't make semantic sense. ".primary-block" should just be ".block .primary".
Additionally, changing this now will break anyone's custom themes who already use Local task blocks.
The selectors of themes not being specific enough and assuming that these should always be horizontal tabs is the real issue, but it's one we can't solve...
I agree this is an issue we should fix, but I'll need some serious convincing before I'd commit this.
Perhaps we should just some more reset-y default CSS?
#4
well, as you can see I sad "something like" what means "I would like to leave the decision by the maintainers or community what the class names should be"
Semantic sense is an oxymoron in this case here, lotyrin. Semantic web has nothing to do with sense making when it stucks on a nonsense solution and describes rather a certain preference over another. And your suggestion brings exactly the problem back, we came from since you have the .primary allone in there again, which could cause confusion with ul.primary depending on when which css file gets loaded. I don't get why primary should be apart here from block, since your module only provide blocks? So there is no semantic question to ask, it would be different if we have a group of primary tab regions to handle, and one of them would be blocks. But there isn't. Well, you will may know.
To be honest, I don't even remember the issue, it's too long ago ... My tracker pinged me. And I rather prefer other solutions now for the "tabs problem" in Drupal, but in general you definitely have a name space collision here with core, there is no doubt about it.
It's a small patch, rather no patch :) heh ... feel free to change the patch to whatever you want.
Greetings
#5
I'm pretty convinced ul.primary, etc are perfectly appropriate classes. Even if they weren't I'd like to avoid changing them at this point.
I think I'll create higher-specificity selectors which style these in a way which makes sense for blocks. (basically reset to what they'd be before the theme's CSS grabs them).
This should only possibly affect existing users using custom themes if they have insufficiently-specific selectors, while also fixing the issue for new users using core themes.
#6
After thinking about this more, I'm changing the status.
It's extremely easy for custom themes to simply NOT assume that ul.primary is to be tabs, when desired. It's easy for a subtheme of a normal theme to override the parent's CSS.
I can also see that the behavior where they are tabs could be desired, making those tabs a block to be placed.