Here is another smörgåsbord* patch that does a ton of things at once. Yay for being in contrib and being allowed all the kittens I can eat! (And yay for the FF spell-checker that tells me how to accentuate smörgåsbord).

Functionality and especially default settings are changed severely, but I'm unapologetic because this is an unstable branch and people were so unhappy with the existing settings that we have nowhere to go but up. :)

Functionality
- Two new features to appease the flood of "it's broken" comments. "Hover" (documented in #300106: Expand on mouse hover.), and "Open". The latter takes the entirely new approach that very few people want to actually close an item that is expanded - and makes the link static once the menu is opened.
- The feature of expanded items being saved in a cookie and being re-expanded on the next page is now optional. It is also automatically disabled when using the "hover" feature, as it makes no sense there.
- Some hopefully saner default settings. The new "Open" feature replaces the "Fake link" feature as the default (which caused about four separate bug reports over the last month alone). To avoid the clutter that accompanies not being able to close menus, the "Close siblings" effect is enabled by default. The cookie option (see above) is disabled by default for the same reason.

Usability
- The fact that DHTML Menu offers a wide array of customization options is now indicated in a message when the module is enabled. Many people complained about functionality without even finding the settings page.

Code structure
- In anticipation of the core module split (#345118: Performance: Split .module files by moving hooks) which was already done by DHTML Menu last year, dhtml_menu.rebuild.inc is renamed to the core standard of dhtml_menu.registry.inc.
- To further improve code-loading, the theming functions that generate menu markup are all split from dhtml_menu.module to dhtml_menu.theme.inc. Menu blocks are generated on many pages, but not necessarily all.
- To reduce variable clutter, all DHTML Menu *behavior* options (other than the list of disabled menus) are stored in one array.
- To centralize the default settings, the settings variable is populated in hook_install(), and all other variable_get() no longer pass a default value.
- All code files are strictly conforming to code style.

Strings
- Many strings were added, more have been adjusted for better comprehension. This means that all current translation files are severely incomplete. However, once this major change has been committed, I promise there will be a string freeze so the translators can work in peace.

Comments

cburschka’s picture

StatusFileSize
new50.11 KB

Against my better judgement, here is another update on top of the existing one. :P However, note that these changes will probably be committed incrementally in small bites, to make the version history clearer. The rebuild->registry renaming definitely won't happen until the core module split.

Bug fix
- With this patch, DHTML Menu finally takes a complete "hands-off" approach to menus that it is not supposed to work with. So far, it was still adding its own markup (classes and IDs); it should now ignore the menus completely.
- JS selectors have been made stricter to ensure that no other module can accidentally trigger them by emitting markup similar to that of core menus. The selectors now require a dhtml-menu class, ensuring that only the markup generated by this module is processed.

Functionality
- Another neat feature is an improved granularity on disabling DHTML. Instead of only allowing a "blacklist", the setting now mimics that of many other filters in Drupal by allowing the choice between a "blacklist" and a "whitelist". You can now switch DHTML on for only the management menu, then add as many new menus as you want without having to disable each one anew.

Usability
- On the list of menus to disable, the settings page now makes a best effort to display human-readable names for system menus, custom menus and books. So far, only books were made human-readable, and this was done by loading the root nodes and caching the titles - now we use book_get_books() if book is enabled.

Code structure
- All options (including disabled menus) are stored in the same settings array. This also means that the list of disabled menus is now included in the Javascript scope - although it is not needed so far, it cannot hurt to have it.
- drupal_static is now used for static caching, and some static caching (eg. for system variables) has been eliminated as superfluous. The static keyword is still used for non-cache variables that must not be reset, eg. the menu router stack or the set of unique IDs.

Strings
- Further changes were made to the settings page strings. These changes are far from done, so translators should definitely wait for the next release candidate before working on those po files.

cburschka’s picture

Status: Needs review » Needs work

The two bug fixes mentioned in #1 are regressions (in a loose sense) in that they have been fixed in D6 by now but never ported up. These have been committed separately now in #352005: Ubercart DHTML conflict - use module-specific selector and #332947: Do not touch menus where DHTML is disabled.. This patch will therefore need a re-roll.

Before that happens, the theme functions will be refactored into theme.inc. Committing that change first (also part of this patch) will make it a lot easier to reroll.

cburschka’s picture

Status: Needs work » Needs review
StatusFileSize
new43.18 KB

With the theme refactoring committed, here is a reroll.

A list of the things it changes, with the changes already committed crossed out. This patch likely won't be committed at all, but it helps to keep track of what needs to be done, and how to resolve editing conflicts.

Functionality
- Two new features to appease the flood of "it's broken" comments. "Hover" (documented in #300106: Expand on mouse hover.), and "Open". The latter takes the entirely new approach that very few people want to actually close an item that is expanded - and makes the link static once the menu is opened.
- The feature of expanded items being saved in a cookie and being re-expanded on the next page is now optional. It is also automatically disabled when using the "hover" feature, as it makes no sense there.
- Another neat feature is an improved granularity on disabling DHTML. Instead of only allowing a "blacklist", the setting now mimics that of many other filters in Drupal by allowing the choice between a "blacklist" and a "whitelist". You can now switch DHTML on for only the management menu, then add as many new menus as you want without having to disable each one anew.
- Some hopefully saner default settings. The new "Open" feature replaces the "Fake link" feature as the default (which caused about four separate bug reports over the last month alone). To avoid the clutter that accompanies not being able to close menus, the "Close siblings" effect is enabled by default. The cookie option (see above) is disabled by default for the same reason.

Usability
- The fact that DHTML Menu offers a wide array of customization options is now indicated in a message when the module is enabled. Many people complained about functionality without even finding the settings page.
- On the list of menus to disable, the settings page now makes a best effort to display human-readable names for system menus, custom menus and books. So far, only books were made human-readable, and this was done by loading the root nodes and caching the titles - now we use book_get_books() if book is enabled.

Code structure
- In anticipation of the core module split (#345118: Performance: Split .module files by moving hooks) which was already done by DHTML Menu last year, dhtml_menu.rebuild.inc is renamed to the core standard of dhtml_menu.registry.inc.
- To further improve code-loading, the theming functions that generate menu markup are all split from dhtml_menu.module to dhtml_menu.theme.inc. Menu blocks are generated on many pages, but not necessarily all.
- To reduce variable clutter, all DHTML Menu *behavior* options (other than the list of disabled menus) are stored in one array.
- To centralize the default settings, the settings variable is populated in hook_install(), and all other variable_get() no longer pass a default value.
- All code files are strictly conforming to code style.

Strings
- Many strings were added, more have been adjusted for better comprehension. This means that all current translation files are severely incomplete. However, once this major change has been committed, I promise there will be a string freeze so the translators can work in peace.

Bug fix
- With this patch, DHTML Menu finally takes a complete "hands-off" approach to menus that it is not supposed to work with. So far, it was still adding its own markup (classes and IDs); it should now ignore the menus completely.
- JS selectors have been made stricter to ensure that no other module can accidentally trigger them by emitting markup similar to that of core menus. The selectors now require a dhtml-menu class, ensuring that only the markup generated by this module is processed.

cburschka’s picture

Status: Needs review » Needs work

A D7 port of #356495: Nice menus compatibility should be incorporated into this.

#477978: Use human readable menu names in the "Disable DHTML" list has now been committed and needs to be removed.

cburschka’s picture

Status: Needs work » Needs review
StatusFileSize
new44.66 KB

Functionality
- #300106: Expand on mouse hover.
- "No collapsing" option takes the entirely new approach that very few people want to actually close an item that is expanded - and makes the link static once the menu is opened.
- The feature of expanded items being saved in a cookie and being re-expanded on the next page is now optional. It is also automatically disabled when using the "hover" feature, as it makes no sense there.
- Improved granularity on disabling DHTML. Instead of only allowing a "blacklist", the setting now mimics that of many other filters in Drupal by allowing the choice between a "blacklist" and a "whitelist". You can now switch DHTML on for only the management menu, then add as many new menus as you want without having to disable each one anew.
- Some hopefully saner default settings. (Planning to do some UX polling on g.d.o)

Usability
- The fact that DHTML Menu offers a wide array of customization options is now indicated in a message when the module is enabled. Many people complained about functionality without even finding the settings page.
#477978: Use human readable menu names in the "Disable DHTML" list

Code structure
- Rename dhtml_menu.rebuild.inc to dhtml_menu.registry.inc.
#477988: Refactor theme functions out of .module
- All DHTML Menu options are stored in one array.
- To centralize the default settings, the settings variable is populated in hook_install(), and all other variable_get() no longer pass a default value.
- All code files are strictly conforming to code style.

Strings
- Many strings were added, more have been adjusted for better comprehension.

Bug fix
- #332947: Do not touch menus where DHTML is disabled.
- #352005: Ubercart DHTML conflict - use module-specific selector The selectors now require a dhtml-menu class, ensuring that only the markup generated by this module is processed.
- #356495: Nice menus compatibility: Instead of depending on recursive theme calls to menu_item_link and menu_item to represent the correct menu structure (which breaks when menu_item_link is called anywhere else), we now index the menu tree's ancestor paths.

cburschka’s picture

Status: Needs review » Needs work

#479260: Solved: Clickable icons changed all the Javascript. Severe reroll/rewrite needed.

cburschka’s picture

Status: Needs work » Needs review
StatusFileSize
new51.93 KB

Patch grew in size again. Partly due to the crazy amount of Javascript fixes.

cburschka’s picture

Status: Needs review » Needs work
cburschka’s picture

Status: Needs work » Needs review
StatusFileSize
new48.54 KB

This patch continues the tradition of "remove one changeset, add one changeset."

Out: Consolidated variables
In: Rename "pseudo-child" to "clone" (and assorted related changes to JS/CSS).

cburschka’s picture

Status: Needs review » Needs work

Back from the summer break!

Okay, first this has to be rerolled again to apply to HEAD. #300106: Expand on mouse hover. is now in and breaks this patch.

cburschka’s picture

Status: Needs work » Needs review
StatusFileSize
new48.65 KB

Heh. It appears I already merged the changesets, but never uploaded the patch. Dropping everything for two months can do that.

cburschka’s picture

Status: Needs review » Needs work

#356495: Nice menus compatibility is committed. Some changes to its changes are still required, which are left in this patch. However, most of the changeset has to be merged out of this patch.

cburschka’s picture

Status: Needs work » Needs review
StatusFileSize
new46.43 KB

And here we go.

cburschka’s picture

No-collapsing has been isolated into its own patch. So here's the new list of changesets included here, including issues that have been closed already:

Functionality
- #300106: Expand on mouse hover.
- #554030: Feature: Non-collapsible menus.
- The feature of expanded items being saved in a cookie and being re-expanded on the next page is now optional. It is also automatically disabled when using the "hover" feature, as it makes no sense there.
- Improved granularity on disabling DHTML. Instead of only allowing a "blacklist", the setting now mimics that of many other filters in Drupal by allowing the choice between a "blacklist" and a "whitelist". You can now switch DHTML on for only the management menu, then add as many new menus as you want without having to disable each one anew.
- Some hopefully saner default settings. (Planning to do some UX polling on g.d.o)

Usability
- The fact that DHTML Menu offers a wide array of customization options is now indicated in a message when the module is enabled. Many people complained about functionality without even finding the settings page.
#477978: Use human readable menu names in the "Disable DHTML" list

Code structure
- Rename dhtml_menu.rebuild.inc to dhtml_menu.registry.inc.
#477988: Refactor theme functions out of .module
- All DHTML Menu options are stored in one array.
- To centralize the default settings, the settings variable is populated in hook_install(), and all other variable_get() no longer pass a default value.
- All code files are strictly conforming to code style.

Strings
- Many strings were added, more have been adjusted for better comprehension.

Bug fix
- #332947: Do not touch menus where DHTML is disabled.
- #352005: Ubercart DHTML conflict - use module-specific selector The selectors now require a dhtml-menu class, ensuring that only the markup generated by this module is processed.
- #356495: Nice menus compatibility: Instead of depending on recursive theme calls to menu_item_link and menu_item to represent the correct menu structure (which breaks when menu_item_link is called anywhere else), we now index the menu tree's ancestor paths.

cburschka’s picture

Status: Needs review » Needs work

#554030: Feature: Non-collapsible menus. has been committed. Needs a reroll to remove the changeset.

cburschka’s picture

Status: Needs work » Needs review
StatusFileSize
new46.29 KB

Merged onto HEAD again.

cburschka’s picture

The optional cookie is now in a separate issue. Also, I forgot I had isolated the variable consolidation and committed it on June 11 already. Here's the list of changesets again.

Functionality
- #300106: Expand on mouse hover..
- #554030: Feature: Non-collapsible menus.. When this is chosen, clicking any expanded menu item wil work like a static link, rather than collapsing the menu again.
- #555500: Make the state-cookie optional. The feature of expanded items being saved in a cookie and being re-expanded on the next page is now optional. It is also automatically disabled when using the "hover" feature, as it makes no sense there.
- Improved granularity on disabling DHTML. Instead of only allowing a "blacklist", the setting now mimics that of many other filters in Drupal by allowing the choice between a "blacklist" and a "whitelist". You can now switch DHTML on for only the management menu, then add as many new menus as you want without having to disable each one anew.
- Some hopefully saner default settings. (Planning to do some UX polling on g.d.o)

Usability
- The fact that DHTML Menu offers a wide array of customization options is now indicated in a message when the module is enabled. Many people complained about functionality without even finding the settings page.
#477978: Use human readable menu names in the "Disable DHTML" list

Code structure
- Rename dhtml_menu.rebuild.inc to dhtml_menu.registry.inc.
- #477988: Refactor theme functions out of .module
- #478472: Consolidate system variables, defaults All DHTML Menu options are stored in one array. To centralize the default settings, the settings variable is populated in hook_install(), and all other variable_get() no longer pass a default value.
- All code files are strictly conforming to code style.

Strings
- Many strings were added, more have been adjusted for better comprehension.

Bug fix
- #332947: Do not touch menus where DHTML is disabled.
- #352005: Ubercart DHTML conflict - use module-specific selector The selectors now require a dhtml-menu class, ensuring that only the markup generated by this module is processed.
- #356495: Nice menus compatibility: Instead of depending on recursive theme calls to menu_item_link and menu_item to represent the correct menu structure (which breaks when menu_item_link is called anywhere else), we now index the menu tree's ancestor paths.

cburschka’s picture

Status: Needs review » Needs work

#555500: Make the state-cookie optional has been committed. Reroll needed.

cburschka’s picture

Status: Needs work » Needs review
StatusFileSize
new45.72 KB

Here's the next one. We're almost out of features and down to code-style now.

The next alpha release will follow when this pipeline is cleared.

cburschka’s picture

Functionality
- #300106: Expand on mouse hover..
- #554030: Feature: Non-collapsible menus.. When this is chosen, clicking any expanded menu item wil work like a static link, rather than collapsing the menu again.
- #555500: Make the state-cookie optional. The feature of expanded items being saved in a cookie and being re-expanded on the next page is now optional. It is also automatically disabled when using the "hover" feature, as it makes no sense there.
- #556042: Better option granularity: Toggle between "blacklist" and "whitelist". Improved granularity on disabling DHTML. Instead of only allowing a "blacklist", the setting now mimics that of many other filters in Drupal by allowing the choice between a "blacklist" and a "whitelist". You can now switch DHTML on for only the management menu, then add as many new menus as you want without having to disable each one anew.
- Some hopefully saner default settings. (Planning to do some UX polling on g.d.o)

Usability
- The fact that DHTML Menu offers a wide array of customization options is now indicated in a message when the module is enabled. Many people complained about functionality without even finding the settings page.
#477978: Use human readable menu names in the "Disable DHTML" list. Instead of displaying the machine names, the human titles are now pulled from menu_get_names as well as book_get_books.

Code structure
- Rename dhtml_menu.rebuild.inc to dhtml_menu.registry.inc. Rendered irrelevant.
- #477988: Refactor theme functions out of .module
- #478472: Consolidate system variables, defaults All DHTML Menu options are stored in one array. To centralize the default settings, the settings variable is populated in hook_install(), and all other variable_get() no longer pass a default value.
- All code files are strictly conforming to code style.

Strings
- Many strings were added, more have been adjusted for better comprehension.

Bug fix
- #332947: Do not touch menus where DHTML is disabled.
- #352005: Ubercart DHTML conflict - use module-specific selector The selectors now require a dhtml-menu class, ensuring that only the markup generated by this module is processed.
- #356495: Nice menus compatibility: Instead of depending on recursive theme calls to menu_item_link and menu_item to represent the correct menu structure (which breaks when menu_item_link is called anywhere else), we now index the menu tree's ancestor paths.

Chasing core
[#547690]. Calls to drupal_function_exists() have been reverted to f_e(), .theme.inc is now required from .module, .rebuild.inc functions merged into .module.

cburschka’s picture

Status: Needs review » Needs work

Committed #556042: Better option granularity: Toggle between "blacklist" and "whitelist".

Also, I now have the enviable task of ripping out the meticulously planned registry stuff again.

This is the last time I will work on compatibility before a core release. The reward for getting things done early is getting to undo them tomorrow. :/

cburschka’s picture

Status: Needs work » Needs review
StatusFileSize
new45.04 KB

Merged out that changeset, now introduced some changes to do away with the registry instead. rebuild.inc is back in .module rather than registry.inc, files and paths are added to t_r_alter, and drupal_ has been removed from all calls to d_function_exists. I anticipate multiple problems with getting custom theme functions to load, but to be quite honest I feel like crossing that bridge when I come to it.

cburschka’s picture

StatusFileSize
new42.08 KB

Registry removal went in at #574690: HEAD compatibility: Remove function registry. Patch rerolled.

cburschka’s picture

Updated. We appear to be nearly done except for strings, code-style, and defaults.

Functionality
- #300106: Expand on mouse hover..
- #554030: Feature: Non-collapsible menus.. When this is chosen, clicking any expanded menu item wil work like a static link, rather than collapsing the menu again.
- #555500: Make the state-cookie optional. The feature of expanded items being saved in a cookie and being re-expanded on the next page is now optional. It is also automatically disabled when using the "hover" feature, as it makes no sense there.
- #556042: Better option granularity: Toggle between "blacklist" and "whitelist". Improved granularity on disabling DHTML. Instead of only allowing a "blacklist", the setting now mimics that of many other filters in Drupal by allowing the choice between a "blacklist" and a "whitelist". You can now switch DHTML on for only the management menu, then add as many new menus as you want without having to disable each one anew.
- Some hopefully saner default settings. (Planning to do some UX polling on g.d.o)

Usability
- The fact that DHTML Menu offers a wide array of customization options is now indicated in a message when the module is enabled. Many people complained about functionality without even finding the settings page.
#477978: Use human readable menu names in the "Disable DHTML" list. Instead of displaying the machine names, the human titles are now pulled from menu_get_names as well as book_get_books.

Code structure
- Rename dhtml_menu.rebuild.inc to dhtml_menu.registry.inc. Rendered irrelevant.
- #477988: Refactor theme functions out of .module
- #478472: Consolidate system variables, defaults All DHTML Menu options are stored in one array. To centralize the default settings, the settings variable is populated in hook_install(), and all other variable_get() no longer pass a default value.
- All code files are strictly conforming to code style.

Strings
- Many strings were added, more have been adjusted for better comprehension.

Bug fix
- #332947: Do not touch menus where DHTML is disabled.
- #352005: Ubercart DHTML conflict - use module-specific selector The selectors now require a dhtml-menu class, ensuring that only the markup generated by this module is processed.
- #356495: Nice menus compatibility: Instead of depending on recursive theme calls to menu_item_link and menu_item to represent the correct menu structure (which breaks when menu_item_link is called anywhere else), we now index the menu tree's ancestor paths.

Chasing core
#574690: HEAD compatibility: Remove function registry. Calls to drupal_function_exists() have been reverted to f_e(), .theme.inc is now required from .module, .rebuild.inc functions merged into .module.

cburschka’s picture

StatusFileSize
new4.3 KB

Isolated code-style for dhtml_menu.theme.inc into this patch.

cburschka’s picture

StatusFileSize
new38.62 KB

Code style for dhtml_menu.theme.inc committed. Here's the rest of the changeset again.

cburschka’s picture

StatusFileSize
new2.28 KB

Merged out the CSS code-style changes as well.

cburschka’s picture

StatusFileSize
new37 KB

CSS changes committed. This is the changeset from the bzr repo again, synched with CVS.

cburschka’s picture

StatusFileSize
new2.03 KB

Breaking the JS changes into isolated patches is nasty work, but necessary in order to maintain proper version control. I want each of these changes to show up in the history, properly labeled.

Already committed one tiny patch of fixed JS comments plus code-style. Here's another that improves on the animation handling: animation settings are populated initially, and then later just passed straight to jQuery.animate().

cburschka’s picture

StatusFileSize
new35.01 KB

Synched. Here's the big patch, once more. Shrinking steadily...

cburschka’s picture

StatusFileSize
new34.47 KB

Committed some whitespace removal and the reversal of the if-condition's branches in switchMenu (it's now if(open), not if(!open)). Shrunk the main patch again.

cburschka’s picture

StatusFileSize
new29.34 KB

Committed several additional comments and slight refactorings. Also reverted a rather questionable flow-control change in the new version. Changeset is down to 30k now.

cburschka’s picture

StatusFileSize
new2.61 KB

Code-style fixes for dhtml_menu.admin.inc isolated.

cburschka’s picture

StatusFileSize
new21.97 KB

Isolated and committed several additional JS changes. Down to 2-3 logical changes and the string update.

After that, I'll branch DRUPAL-6--4, before focusing on updating the theme API-related stuff in HEAD.

cburschka’s picture

StatusFileSize
new0 bytes

Here's the last patch, containing only the new strings (and new .pot file). This will be committed as one piece.

cburschka’s picture

StatusFileSize
new15.56 KB

Bad diff.

cburschka’s picture

Status: Needs review » Fixed

Committed.

Next week: Branching and fixing compatibility with 6.x as well as 7.x.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.