UR-4862: Fix multi-step form rendering step 1 fields inside step 2 - #1404
Open
saurab018 wants to merge 1 commit into
Open
UR-4862: Fix multi-step form rendering step 1 fields inside step 2#1404saurab018 wants to merge 1 commit into
saurab018 wants to merge 1 commit into
Conversation
Rows are identified by a creation-order id that never renumbers, while user_registration_form_row_ids stores them in visual order. The first row of every form is hardcoded to id 0, so as soon as any later-created row is dragged above it, row 0 leaves index 0. At that point `! empty( $row_ids[ $index ] )` evaluates false for row 0 (both int 0 and the string "0" that the meta actually stores) and the row silently falls back to `$index`, stamping it with another row's id. The multi-step addon then wraps it in whichever part owns that id, and row level conditional logic matches the wrong row for the same reason. Guard on the array instead of the element so a legitimate row id of 0 is kept, matching the pattern already used in class-ur-users-menu.php.
Contributor
|
Build for ⬇️ Download user-registration-5.2.7.zip (8.1M) Installs directly via Plugins → Add New → Upload Plugin. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Multi-step registration forms rendered step 1's fields inside step 2. On a reported form the first step showed only its section heading while every personal field (First Name → Mobile Phone) appeared on step 2 above the employment fields, even though the step configuration in the builder was correct.
Row ids are creation-order identifiers that never renumber, while
user_registration_form_row_idsstores rows in visual order. The builder hardcodes the first row of every form todata-row-id="0", so dragging any later-created row above it (adding a Section Title header, for example) moves row0off index 0. At that point! empty( $row_ids[ $index ] )is false for row0— the meta stores ids as strings, and"0"is falsy too — and the row silently falls back to$index, stamping it with a different row's id. The multi-step addon then wraps it in whichever part owns that id. Rows now keep their real id regardless of position, so each one lands in its configured step.Changes proposed in this Pull Request:
templates/form-registration.php— the row id guard changes from! empty( $row_ids[ $index ] )to! empty( $row_ids ) && isset( $row_ids[ $index ] ). The$indexfallback is preserved for legacy forms that have nouser_registration_form_row_idsmeta at all, which was the fallback's actual purpose; it just no longer swallows a legitimate id of0.Multi-step addon unchanged —
display_before_row()already resolves parts correctly viain_array( $row_id, $rows, true ). It was acting on a corrupted$row_id, not choosing the wrong part.Stored form data unchanged — no migration. The multi-step configuration was already correct on affected forms; only the rendered
data-row-idwas wrong.Note
This is the only place the bug can occur. The three other consumers of
$row_ids—class-ur-users-menu.php:892,class-ur-members-menu.php:1631andtemplates/myaccount/form-edit-profile.php:364— already guard with! empty( $row_ids ) && isset( $row_ids[ $index ] ); this template was the lone straggler, and the fix adopts their existing pattern rather than inventing one. Row-level conditional logic a few lines below compares$row_id == $individual_row_data->row_idand was misfiring on the same corrupted value, so it is fixed by the same line.Row id resolution
flowchart LR A["Render row<br/>at visual index N"] --> B{"row_ids meta<br/>present?"} B -- no --> C["Fall back to<br/>$index"] B -- yes --> D{"Changed:<br/>isset row_ids[N]?"} D -- no --> C D -- yes --> E["Changed:<br/>use real row id<br/>including 0"] E --> F["data-row-id correct,<br/>row lands in its<br/>configured step"] C --> F D:::changed E:::changed classDef changed fill:#9a6700,color:#fff,stroke:#5c3d00,stroke-width:3px,stroke-dasharray:6 3How to test the changes in this Pull Request:
Create a registration form with two rows. Put username/name fields in the first row, password fields in the second. Save.
Add a new row, drag it above the existing first row, and drop a Section Title field into it. Save. This is what makes row
0stop being the first row — confirm it with:Expect: the array does not start with
"0"— e.g.["2","0","1"].Enable Page Options (multi-step), create two steps, and assign the rows so that row
0's fields belong to step 1.View the form (
/?ur_preview=true&form_id=FORM_ID) and inspect the rendered rows:Expect: every id is distinct and
"0"is present. Before the fix this printed a duplicate id and no"0"— the row whose real id was0was stamped with the id of the row sitting at its index.Check which step each row landed in:
Expect: row
0's fields sit inuser-registration-part-1. Before the fix they sat in part 2, and step 2 could render with no fields of its own at all.Fill step 1 and click Next.
Expect: step 1 hides, step 2 shows, and step 2 contains only its configured fields.
Negative case — open a form that has never had its rows reordered (
row_idsstarts at"0", or the meta is absent entirely).Expect: unchanged rendering; ids still ascend from
0and every step holds the same fields as before this patch.Regression check on row conditional logic — on the reordered form from step 2, set a conditional rule on a row and trigger it.
Expect: the intended row shows/hides. Before the fix the rule applied to whichever row shared the corrupted id.
Types of changes:
Other information:
Verified with a controlled A/B on a local WordPress install (UR Pro 6.2.7, Multi-Step 1.3.2) against a form with row id
1at visual index 0 and row id0at index 1, parts configured aspart1=[1],part2=[0]. The template line was the only variable changed between runs:No automated test was added — the affected code is inline markup in a template with no existing test harness covering it.
Changelog entry
Fix - Multi-step registration form rendering the first step's fields inside the second step when form rows had been reordered.
Ref: UR-4862