Chase core's lovely new theme API
| Project: | DHTML Menu |
| Version: | 7.x-1.x-dev |
| Component: | PHP Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Good news everyone! The theming API, and especially the menu part of it, has changed drastically enough to require a big rewrite in DHTML Menu.
Why is it good?
Because the way DHTML Menu has been doing this ever since 6.x-3.0 has, frankly, been iffy. I know it was my idea, and it seemed like a good idea at the time (and look at how well it's worked), but there have been lots of consequences and troubles that the module would be better without. Intercepting theme functions is nasty work.
Now we have:
- one single theme function, menu_link, instead of two. And they don't call themselves in some mad recursion either.
- the theme API honors preprocessors for theme functions
- the preprocessor gets the whole menu item stuffed in a handy little key called #original_link.
It's like the menu and theme layer gurus got together and conspired to make my life easier. THANK YOU! :D
Now that DHTML Menu 6.x-4.x is branched and well under way, it's time to chase core again by working with this new theming concept.
#1
Here's a first patch. As shown, we can cut over a hundred lines of code through this.
Javascript had to be updated too, and will require more changes for stable functionality. After applying this patch, I was able to install DHTML Menu on HEAD and expand menus using the default settings.
#2
This patch works great. Thanks a lot.
Do you think it's possible to handle pull-down menus as well eventually?
#3
If you mean menus similar to the ones NiceMenu produces, probably not in the forseeable future. Don't get me wrong, I like them and combine NiceMenu with DHTML Menu on one of my sites, but implementing it in this module would require time I don't have right now.
#4
This patch is committed to HEAD and will appear in the dev snapshot within 24 hours.
There will be another alpha release once I've looked the code over a bit more.
#5
Yes, that'S what I meant. The problem with nice_menus is that (1) it's not very active for at least 6 months and (2) there is no sign for a D7 port. However, I do appreciate your time constraints and wonder if you were interested if we jumped in and provided some of the work to be then contributed to your module.
My thinking was that your D7 infrastructure looks really solid and as a perfect starting point for adding drop-down menus as well. If you were interested I'm happy to discuss the requirements and code design guidelines in a seperate thread or offline, please let me know.
#6
@Arancaytar, please have a look at this patch. It's an implementation of drop-down menus without breaking any of your existing functionality. The CSS coding is just basic at this point and needs more work but I wanted you to have a look at this approach first.
This is how it works: only the JS file is altered and as soon as a dhtml-menu is wrapper ba a drop-down class it handles just that menu as if the module settings was 'hover'. That's all. Nothing else required.
What do you think?
#7
Hey @Jürgen, thank you very, very much for this patch! I haven't had a chance to try it out yet, but it looks quite clean and straight-forward.
I hope to get around to reviewing it in detail next weekend (21.-22.11.)
#8
Hi @Arancaytar, thanks for your attention on this one. Here is another improved patch which is the better one for your review. It applies extra classes to nested and deep ul's in the drop-down menu so that styling becomes possible. The CSS contains a default which supports drop-downs out-of-the-box.
Looking forward to your feedback.
#9
I like the way the drop-down menus look, but so far there doesn't seem to be any code for actually enabling the option - I had to hack the drop-down class into the markup manually. Also, the menu seems to disappear before I can select anything from it, because it's only visible while my mouse is over the parent item (this may be caused by me adding the class incorrectly).
By the way, could you post this as a new feature request, please? This issue is about the theme API compatibility, and should technically be closed now.
#10
Agreed, this is followed up in #641720: Support for Drop-Down menus and I briefly explain there how to implement the drop-down functionality in a theme.