Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Putting a multifield inside a multipage field group results with Move submit button to last multipage enabled results in the multifield's "Remove" button being moved out of the multifield and onto the last page of the form.
Steps to reproduce
- Install a fresh copy of Drupal 7.37 with the minimal install profile.
- Download
multifield-7.x-1.0-alpha1
and it's dependency,ctools-7.x-1.7
. - Download the latest version of
field_group-7.x-1.x-dev
. - Log in to the new site as user/1.
- From
admin/modules
, enable the Field UI, Chaos tools, Field Group, and Multifield modules. - From
admin/structure/types/add
, add a new content type. I named mine "Test". - Go to the Manage fields tab for that content type (mine was at
admin/structure/types/manage/test/fields
). There should already be a "Title" and "Body" field in the content type. - Add a new field of type "Multifield" (I named mine "Multifield 1"). Use the default settings, but set the "Number of values" (a.k.a. "Cardinality") to "Unlimited".
- When the Multifield has been created, click "Manage subfields" in the "Field type" column.
- Add a new field of type "Text" (I named mine "Textfield a"). Use the default settings.
- Go to the
node/add
page for the new content type (mine was atnode/add/test
).
Note there is a "Remove" button inside the multifield control. - Return to the Manage fields tab for your content type.
- Add a new "multipage group" group (I named mine "Multipage group i"). After it's created, click the gear in the Operations column, and set Move submit button to last multipage to "Yes".
- Add two "multipage" groups (I named mine "First page" and "Second page").
- Move "First page" and "Second page" inside "Multipage group i", in that order (i.e.: first page is first).
- Move the "Body" field inside "Second page".
- Move the "Multifield 1" inside "First page".
- Go to the
node/add
page for the new content type (mine was atnode/add/test
).
Note there the "Remove" button is missing from the multifield control. - Click "Next page" to go to the next page. Note there is a "Remove" button at the bottom of the second page that doesn't seem to be connected to anything.
Analysis
This happens because:
- the
form_process_actions()
give theform-actions
class to all FAPI elements of typeactions
, - this module's
multipage/multipage.js
moves ALL elements that match the selector.form-actions
to the last page of the multipage when Move submit button to last multipage is enabled, - multifield sub-forms contain a form API element of type
actions
— which kind-of makes sense considering they could be displayed on a page by themselves if JavaScript is disabled
Proposed resolution
I propose:
- Going through the first level of the form array, searching for any form elements of type
actions
. If we find one, we give it the classfield_group_form_actions
. - Changing
multipage/multipage.js
to only move form elements with thefield_group_form_actions
class.
The reasoning behind this is:
- It seems that the idea behind the Move submit button to last multipage option is to make it hard for users to accidentally submit the whole form before it's done,
- It's reasonably safe to assume that the first level of the form array contains items for the whole form — put another way, it's more likely that any actions elements deeper than the first level belong to a sub-form,
- If someone has a special case where they've nested the form's actions somewhere, they can "opt-into" moving the actions to the last page by adding the
field_group_form_actions
class — while less automatic, it reduces the chance that multipage will accidentally affect something in more-common cases (e.g.: using multifield)
Remaining tasks
Write a patch- Review and RTBC
- Commit
User interface changes
Rather than moving all items with the class form_actions
to the last page of the form, only items with the class field_group_form_actions
will be moved instead.
API changes
Adds a field_group_form_actions
class. Items with this class will be moved to the last page of the form.
Comment | File | Size | Author |
---|---|---|---|
#10 | stop_moving_sub_form-2497345-10.patch | 17.83 KB | skylord |
Comments
Comment #1
mparker17Added a link to the issue description, attached a patch, moved to "Needs review", unassigned.
Will add steps to reproduce shortly.
Comment #3
mparker17Added steps to reproduce.
Comment #4
mparker17Invalid patch format in stop_moving_sub_form-2497345-1.patch? That's weird. Maybe it was the mixed line endings in
multipage/multipage.js
? Let's try it with UNIX line endings only.(no interdiff because I only changed the line endings of the last patchfile).
Comment #6
mparker17Very strange.
Let's see what happens if I change all line endings in the JavaScript file... this will make it look as if all lines have changed in the file.
(no interdiff because I only changed the line endings of the javascript file).
Comment #7
mparker17Since elements in a form array don't necessarily have a '#type' element, I've added a check to prevent "undefined element" errors.
Comment #8
skylord CreditAttribution: skylord commentedWorks fine with Paragraphs inside pages! Thanks!
Comment #9
Chris Matthews CreditAttribution: Chris Matthews as a volunteer commentedThe 4 year old patch in #7 to field_group.module does not apply to the latest field_group 7.x-1.x-dev and if still applicable needs a reroll.
Comment #10
skylord CreditAttribution: skylord commentedReroll for last dev.
Comment #11
Chris Matthews CreditAttribution: Chris Matthews as a volunteer commented