Standard Vanilla Drupal 7.8 install
node/add/page has "Main menu" as (only) active menu, no way to select other menu's.

enable hs_menu.module
node/add/page now has "Main menu", "Management", "Navigation" and "User menu" available.

problem: this is faulty behaviour, hs_menu completely ignores the "Available menus" setting defined in admin/structure/types/manage/page

What should happen is hs_menu should replicate what core Menu does,

trying a fix:
What I did in hierarchical_select.module is add the content type to $config['params'] in function form_hierarchical_select_process() when we are dealing with nodes. Next, in hs_menu.module, if a type is present in $params in function hs_menu_hierarchical_select_root_level(), I load the menu_options variable for that content type and thus only load the wanted menu's in the hierarchical select.

(patch created at Coworks)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rv0’s picture

Status: Active » Needs review
Wim Leers’s picture

Status: Needs review » Needs work

Please move the content type detection code from hierarchical_select.module (which should remain general-purpose) to hs_menu.module's hs_menu_form_node_form_alter(). Then this is good to go :)

rv0’s picture

EDIT:ignore patch below this, looking for the correct file

rv0’s picture

Status: Needs work » Needs review
FileSize
2.53 KB

grr, took wrong file ;)

found it:

(patch created at Coworks)

Wim Leers’s picture

Status: Needs review » Needs work

- Why the array_key_exists() calls instead of isset()? isset() is more common in Drupal AFAIK. It's also shorter.
- Indentation is off here:

+  _hs_menu_apply_config($form['parent'], array('exclude' => array(
+      $original_item['menu_name'],
+      $original_item['mlid'],
+      )));

The last line should be indented to the same level as the first level.
- Please add some comments before + if (isset($params['type'])) {

rv0’s picture

about isset():
If I'm not mistaking it will throw warnings in some php configurations (when it checks if a non existing element has a value).
it is faster though.

Will research more thoroughly, I've been doing it like this for a long time, I'm not that worried about "longer" syntax ;)

Wim Leers’s picture

Well, I just want to be consistent about it. I've used isset() everywhere, unless array_key_exists() is absolutely necessary. It'd be strange to start changing that.

Also, in principle, Drupal has the rule that every line should be max 80 chars long. I'm definitely not meeting that rule everywhere, but this change would cause us to go over the 80 chars limit in *a lot* of places.

Wim Leers’s picture

I was waiting for this to be ready before tagging alpha 5. I decided it was more important to put alpha 5 in the hands of many people, so this one is for the next release :)

acbramley’s picture

Can a new patch be generated with the changes suggested in #5?

rv0’s picture

@Zombienaute

It's on my todo list, along with some other patches for HS I need to re-roll
Sadly, the todo list is quite long atm, so no ETA.

wiifm’s picture

Tried out patch #4, and this has solved the issue.

Thanks for your hard work here

rv0’s picture

Status: Needs work » Needs review
FileSize
2.58 KB

Rerolled the patch against the latest git, including the changes proposed in #5

acbramley’s picture

Status: Needs review » Reviewed & tested by the community

Works perfect!

mariomaric’s picture

I can also confirm, patch from #12 works perfect!

Thx rv0!

P.S.
I applied it to alpha5, because it is newer version..

mariomaric’s picture

Category: feature » bug

IMHO this is bug.

And it is not compatible with patch from #1272538: Hierarchical Select Menu should also flatten on submit of node forms issue. :/

agentrickard’s picture

Here's a reroll that applies after applying the patch from #1272538: Hierarchical Select Menu should also flatten on submit of node forms.

@mariomric

In the future, please be more specific than "is not compatible." That phrase could mean:

a) Patch does not apply.
b) Patch applies but breaks functionality.

mariomaric’s picture

Priority: Normal » Major

@agentrickard: Will surely be more specific next time.

Thx for reroll. :)

agentrickard’s picture

Not a problem. I just fine myself being more directive in the queue lately.

Turns out I won't be using HS on this project, so I'm leaving this issue ;-p.

Wim Leers’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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