webform_civicrm
webform_civicrm copied to clipboard
#3095675 Show Line Item in same order as form field are configured
Overview
https://www.drupal.org/project/webform_civicrm/issues/3095675
Before
Line Item table does not follow weight flag of field
After
Line Item table does follow weight flag of field
Technical Details
Line Item table follow weight flag of field
Hi @sunilpawar - thanks for this. Will definitively take a look in near future.
@sunilpawar - I just tested this and ran into some issues. Here's my Webform:
And these are my Line Items afterwards with this PR 278:
Please note -> not only is the order not the same as on the Webform (which is what this PR is attempting), note that Line Item Amount 2 is missing.
If I git stash -> to before 278.diff -> Line Item Amount 2 is there:
Note I've not yet tested multiple Participants or multiple Memberships.
Conclusion: this would certainly be nice but in order to make this mergeable (after fixing the ordering and after handling multiple line items) please show some testing with (in addition to what I just did) multiple Memberships and multiple Participants as well.
PS - sorry for the delay on reviewing this. I'm finally catching up.
@KarinG thanks for reviewing PR, i will check multiple scenario and get back to you with additional fix.
Looking at the code here, I have a suggestion.
Although it's very clever to index the array by component weight and then ksort()
at the end, it results in overall more complex code, as the $formFieldOrder
variable has to be used everywhere a line item is added to the array. Seems to make the code harder to read & understand.
I would suggest that instead, you index the line-item array by $fid
and then add a utility function to do the reordering at the end. That function would look up component weights by $fid
and then ksort
. That function should also involve a fallback in case the component is undefined.
To prevent items disappearing, you could also suffix the array keys like [$fid . '-a']
and [$fid . '-b']
. The utility function would just need to be smart enough to handle that.