In subform_wrapper_after_build() there is a code that allows implementing modules to set the triggering element manually. However it doesn't work, as we need to set also the $form_state, not only the $subform_state.
Also when we try to get the nested value, we should do it from the $form_state and not the $subform_state.

Current patch fixed that.

To test, you can use user-modal (in my sandox) and try in FF to login using the Enter key. User modal, makes sure the triggering element is the current tab the user is watching.

CommentFileSizeAuthor
#2 1186194-fix-trigger-2.patch2.72 KBamitaibu
allow-setting-trigger.patch2.79 KBamitaibu

Comments

casey’s picture

casey commented

Hmm the whole pattern is a bit flawed. What if you have multiple subforms that all need a different triggering element? Then #subform_triggering_element wouldn't be sufficient.

Maybe we should just drop #subform_triggering_element. If you want manually set a triggering element you could just add an #process handler for that, in which you call subform_set_triggering_element().

The subform_set_triggering_element() function is however fine. We just needed to pass the $subform_state instead of the $form_state.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
StatusFileSize
new 1186194-fix-trigger-2.patch2.72 KB

Here's another stab:

1) Removed the flawed code.
2) Changed signature of subform_set_triggering_element() to include $subform_name.
3) subform_set_triggering_element() Set the triggering element also on the $form_state, as it is later required in subform_element_submit() (and documented that).
4) Not sure about it -- I have removed the part where we set the $subform_state[$type . '_handlers']. Otherwise in user_modal_subform_after_build() when I call subform_set_triggering_element() it results we the submit handler being subform_element_submit() instead of the correct one -- probably needs more attention to understand why it was needed in the first place.

btw, did you have a chance to install user-modal module? at least for me it provides a good test case for subform -- which I'm soooo happy you wrote ;)

casey’s picture

casey commented

Only dropped #subform_triggering_element.

Looking at user-modal I think you had to set the triggering element of the wrapper form, so you're code has to be something like:

function user_modal_subform_after_build($element, &$form_state) {
  $user_action = user_modal_get_user_action($form_state);
  subform_set_triggering_element($element['tabs'][$user_action], $form_state);
  return $element;
}
casey’s picture

casey commented

Also added some doc to subform_set_triggering_element().

casey’s picture

casey commented

Reintroduced #subform_triggering_element. See module page for docs.

casey’s picture

casey commented

I think you should change it to just setting the $form_state['triggering_element']. I am thinking of dropping subform_set_triggering_element().

function user_modal_subform_after_build($element, &$form_state) {
  $user_action = user_modal_get_user_action($form_state);
  $form_state['triggering_element'] = $element['tabs'][$user_action];
  return $element;
}
casey’s picture

casey commented
Status: Active » Fixed

Ok I keep subform_set_triggering_element() for now, but it should only be used on subforms. Also added some extra doc.

Status: Fixed » Closed (fixed)

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