Expand the commerce_profile_select form element with the ability to reuse profiles #760
Conversation
…an: Expand the commerce_profile_select form element with the ability to reuse profiles
| */ | ||
| class ProfileSelect extends RenderElement { | ||
| class ProfileSelect extends FormElement { |
There was a problem hiding this comment.
This is reverting the fix from #2857051.
Might have started by accident, since this PR is older than that fix.
| 'wrapper' => $wrapper_id, | ||
| ], | ||
| '#submit' => [[get_called_class(), 'ajaxSubmit']], | ||
| '#name' => 'edit_profile', |
There was a problem hiding this comment.
I'm testing the patch when enabling commerce_shipping module.
Because, the commerce_shipping also uses the ProfileSelect element, in checkout page, there are multiple submit buttons with same #name. If the edit_button element is clicked, the trigger element will be wrong.
In my testing, when I clicked the "Edit" button, the trigger element was the "Continue to review" button. However, the expected and correct trigger element should be "Edit" button. The problem may result from the same value of #name and more detail is in issue #2546700.
In order to prevent the situation, we should have a workaround for the issue to make the value of #name unique.
My rough implementation:
$form_name = (isset($element['#parents'])) ? implode('_', $element['#parents']) . '_' : '';
$form_name .= 'edit_profile';
And the value of the #name is $form_name.
| '#suffix' => '</div>', | ||
| // Pass the id along to other methods. | ||
| '#wrapper_id' => $wrapper_id, | ||
| '#element_mode' => $form_state->get('element_mode') ?: 'view', |
There was a problem hiding this comment.
If the multiple ProfileSelect form elements exist in the same form (eg. in checkout page, the shipping info and billing info), the element_mode in $form_state should be separated by the form parents.
For example, commerce_shipping module is installed, and there are two form elements in the same checkout form page. Once a edit_button within shipping info (or payment info) is clicked, the same element_mode value in $form_state is set into edit by ajaxSubmit(). Therefore, #element_mode in shipping info and payment info are edit. It caused another form element in payment info doesn't contain edit_button.
Perhaps, the element mode name should be with the form parents.
$form_state->get('element_mode-' . $id_prefix . '-edit_button')
| */ | ||
| public static function ajaxSubmit(array &$form, FormStateInterface $form_state) { | ||
| $triggering_element = $form_state->getTriggeringElement(); | ||
| $form_state->set('element_mode', $triggering_element['#element_mode']); |
There was a problem hiding this comment.
As previous mention, the element_mode key in form state should be changed into:
$parents = implode('-', $triggering_element['#parents']);
$element_mode_name = 'element_mode-' . $parents;
$form_state->set($element_mode_name, $triggering_element['#element_mode']);
|
Hi @bojanz I would like to enhance the PR, should I create a PR into 8.x-2.x or profile_reuse branch? |
|
@ymhuang0808 hm, yeah that's the problem with Github. I'll wait to see what @bojanz says. But to follow up: one of the main issues is that the element does not work if there are multiple elements on the same page. Correct? |
|
@mglaman Yes. I'm tring to fix that. |
|
@bojanz and @mglaman The enhancement for the PR is ready to review on fliegen@57c1117. Please take a look. I think I should create a new PR into profile_reuse branch. The commit has a few changes:
BTW, in order to support the ProfileSelect, I also revised the commerce_shipping. Thanks. |
|
This PR is outdated, the issue has many updated patches. |
Continuation of #631, done by czigor and mglaman.