Closed (fixed)
Project:
Web Links
Version:
6.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
5 Jul 2009 at 18:04 UTC
Updated:
31 Jul 2009 at 13:41 UTC
In the new-look tabs created by weblinks_help, I think the class list also needs 'clear-block', ie change
return '<div class="tabs">'. theme('links', $links, array('class' => 'tabs primary')) .'</div>';
to
return '<div class="tabs">'. theme('links', $links, array('class' => 'tabs primary clear-block')) .'</div>';
The clear-block class is used when tabs are generated in the usual way by menu functions, and I am guessing you intend the new tabs to be rendered in the same way as menu tabs. The first attached image shows what happens in a Zen theme when the clear-block class is omitted.
My weblinks tabs do not look the same as normal menu tabs and I think it is because the regular tabs are rendered using theme('menu_item_link' and theme('menu_local_task' whereas the weblinks code is trying to replicate that with a single call to theme('links'. The active/not active class is not being added correctly and styling which is done in the theme('menu...' functions does not get created. I have attempted some code to replicate the theming of the normal menu tabs. The line above is replaced with:
$output = '<div class="tabs"><ul class="tabs primary clear-block">';
foreach ($links as $name => $link) {
$link['type'] = MENU_LOCAL_TASK;
$output .= theme('menu_local_task', theme('menu_item_link',$link), $path == $link['href']); //3rd param is (active true/false)
}
$output .= '</ul></div>';
return $output;
This produces the tabs as shown in the second image, but I am not happy with the code. The active/not active class is still not working because $path is always 'weblinks'. I am also not too happy about coding out the ul tags. Do you have any ideas about this? I think we should try to make the new tabs render the same as existing menu tabs.
Jonathan
| Comment | File | Size | Author |
|---|---|---|---|
| weblinks_tabs_corrected.jpg | 37.69 KB | jonathan1055 | |
| weblinks_no_clear_block.jpg | 97.85 KB | jonathan1055 |
Comments
Comment #1
nancydruHmm, I see what you're talking about. It's interesting that the two themes I tested with both look so close to menu tabs that most people wouldn't notice. I just tried two other themes and they are really ugly. I didn't really see that "clear-block" helps much.
I think the key is to really rethink the menu path problem and weblinks_director hook. However, I think this is a lower priority because it seems pretty good to me as it is now.
Comment #2
jonathan1055 commentedYes, re-doing it with your new director and using menu paths might be the best approach, but that will take quite a bit of work and planning, so I am guessing that it wont be imminent.
Have you tried my alternative code above in your two ugly renditions? If it makes them look better, (presuming that the regular menu tabs look nice in the first place ;-) would it be worth implementing this as is, but knowing that we might do a big overhaul later. I have tried to replicate the exact source code that is generated by the menu functions so it should look correct on many themes which would appear broken with the code as it stands.
Jonathan
Comment #3
nancydruNo I didn't try your code yet, but I did add the "clear-block." There are some ideas running around in my head, but I have to get some problems fixed for a customer right now.
Comment #4
nancydruChange the title to more reflective of the desire.
Comment #5
nancydruCommitted to -dev. Please test it out thoroughly.
Just to up the ante, I added a "return to links page" link to the settings page.
Comment #6
jonathan1055 commentedOh WOW! first impression: Excellent!
What a big improvement. Yes, using menu path and local tasks is definitely the right way to go. I was wondering how you were going to incorporate all the disparate directory paths in weblinks, and your solution is very nice indeed.
I will do some more testing, and get back to you, but wanted to first say a big thank you.
Jonathan
Comment #7
nancydruThe only "drawback" to this is it makes using a return destination difficult.
Comment #8
jonathan1055 commentedDid some testing and have a few suggestions (only minor/cosmetic/text, but they will make it even better). Not got time to post them now, but will make a patch so you don't have to do manual retyping. I am away tomorrow and the weekend, so will get the patch here during next week.
Jonathan
Comment #9
jonathan1055 commentedSorry to post to fixed issue, but you marked it fixed too quickly ;-) I have some other things to suggest with regard to the latest dev but the two things that apply directly to the re-working of the tabs are:
These are not show-stoppers, so do not require urgent attention, but they will make a great module even better if fixed.
Jonathan
ps I will post the other suggestions with a patch in a new issue.
Comment #10
nancydruI mark it fixed when I commit a fix. The system gives two weeks before closing the issue.
1. I wasn't sure about whether to force its position, so I just let the menu system do it alphabetically. I put the link at the end on the settings to suggest "Make all your changes, then go back." Committed the first part.
2. Hmm, when I edit a group, the menu links disappear entirely. The menu item is a callback because a) I don't want the casual hacker to get to it, b) so it does not show up in the menu tabs since it should not be accessed that way. Try disabling the module (but do NOT uninstall), then re-enable it and see if this goes away. If you have devel, just "Rebuild menus."
Comment #11
nancydruIncluded in 6.x-2.1.
Comment #12
jonathan1055 commentedHi again,
I don't understand, because I posted a reply to the previous point but now it is not here? Maybe after I hit 'submit' the server stopped responding in time for it to be saved. Anyway, here is what I said:
1. I would still prefer the 'links page' tab not to move around, but if you disagree, that is fine, I will just make a patch for myself and apply it each time I upgrade weblinks.
2. I tried disabling then re-enabling the module - no effect, all the the tabs are shown during group edit. I also did 'rebuild menus' from devel and this also had no effect. My theme is based on Zen so it fairly standard and well-used. I tried it with Garland and all the tabs are shown when editing a group. Do your tabs disappear?
Jonathan
Comment #13
nancydruI switched to Garland. On my test system, the tabs move anyway simply because the tab group on the settings page is short enough to fit on the same line with the title but on the links page they do not.
@rmiddle: Should we move the "Links page" tab on the settings page back to first in the list? If we do, when someone goes to settings, the left-most tab would not be highlighted.
Even under Garland I do not see the tabs in group edit, not would I really expect to, since we have actually bounced into the Taxonomy module. I have tried Pixture_reloaded, Garland, Wyo, Bluemarine, Chameleon, and Slash all with the same result.
Comment #14
nancydru