so in line 105 and 107 of local_tasks_block.module

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 

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 ...

CommentFileSizeAuthor
#1 local_tasks_blocks.patch2.53 KBdqd

Comments

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Title: css classes should be different from system classes to have a default fit into blocks » css class should be different from system class (patch needs review)
Status: Active » Needs review
StatusFileSize
new local_tasks_blocks.patch2.53 KB

ok, here's the patch

leahmd’s picture

leahmd commented

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.

lotyrin’s picture

lotyrin commented
Status: Needs review » Needs work

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?

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented

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

lotyrin’s picture

lotyrin commented
Title: css class should be different from system class (patch needs review) » Core theme CSS assumes ul.primary, etc. should be styled as if they were tabs
Assigned: Unassigned » lotyrin

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.

lotyrin’s picture

lotyrin commented
Status: Needs work » Closed (works as designed)

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.

Morten Najbjerg’s picture

Morten Najbjerg commented

Thank you for this module - very handy.
I'm currently in a situation where I need to add my own classes to the unordered list. The UL tags are not available for preprocess - which leaves you with no other options than hacking the module when You need additional classes on the lists.
Would it be possible to provide templates for the rendering of the lists instead? This could also be a solution to this discussion because people are able to add the classes they find necessary.

I'm thinking something like:

<ul class="<?php echo $classes; ?>" <?php echo $attributes; ?>><?php echo $tasks; ?></ul>

Using this method every usecase would be covered as the programmers are able to preprocess the output and add attributes and classes whenever needed.

Morten Najbjerg’s picture

Morten Najbjerg commented
Category: task » feature
Status: Closed (works as designed) » Active

I'm opening this again to hear if the things I have proposed would be useful for other than me :)

jay.dansand’s picture

jay.dansand commented
Status: Active » Closed (works as designed)

@Morten Najbjerg: This looks like it should be a separate issue than the original post, so I'm closing this thread, but a more detailed response is below.

The 1.x branch is no longer actively maintained. I'm the latest project maintainer, having just merged in a different module that solved the problem in a similar fashion. Unfortunately this means I'm not intimately familiar with the original module, and will be concentrating on supporting 2.x.

Please try the 7.x-2.x branch (the new code base) and let me know if it works for you. The 2.x branch uses the Drupal theme layer to create the normal tabs output, and should allow you to alter the output by implementing theme hooks. Unfortunately a lot of the menu_local_tasks theming is hard-coded in Drupal functions (see theme_menu_local_tasks() and theme_menu_local_task()) so it's not as easy as it could be.

My advice would be to try the example ("How to add the colorbox class..." posted by FranciscoLuz) in the theme_menu_local_tasks() documentation.

Best of luck!

jay.dansand’s picture

jay.dansand commented
Issue summary: View changes

two other lines should be changed