Hello,
In https://cgit.drupalcode.org/bootstrap/tree/src/Plugin/ProcessManager.php..., the findButton() method does not find the nearest button as in the method PHPDoc, but the last one, as there is no break in the foreach when finding a button.
I found the problem in a form with multiple submit buttons.
I will provide a patch in a few hours and also testing on the last dev version of the theme.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | bootstrap-preprocess_find_button-2944760-6.patch | 2.61 KB | grimreaper |
| #4 | bootstrap-preprocess_find_button-2944760-4.patch | 2.6 KB | grimreaper |
| #2 | bootstrap_button_3.png | 3.04 KB | grimreaper |
| #2 | bootstrap_button_2.png | 3.86 KB | grimreaper |
| #2 | bootstrap_button_1.png | 3.05 KB | grimreaper |
Comments
Comment #2
grimreaperTested on the last dev version.
There is still the bug.
I have uploaded a fix.
I have done tests modifying directly the native search block form to add submit button. www/core/modules/search/src/Form/SearchBlockForm.php:
bootstrap_button_1.png: the result without the patch.
bootstrap_button_2.png: the result with the patch.
bootstrap_button_3.png: the result with the patch and with the "test 1" button commented out.
Thanks for the review.
Comment #3
markhalliwellInteresting. Cannot believe I missed that heh
The
findButtonmethod really doesn't belong inProcessManageranyway and should instead live in theElementclass instead.I think this patch should move the method (getting rid of the unnecessary
$elementparameter) and then deprecate this existing method?Then it should be a simple as
$button = &$parent->findButton()inprocessInputGroupsComment #4
grimreaperThanks @markcarver for the review.
Here is a new patch with your suggestion.
About the deprecation, I don't think it will be very frequent to have custom theme using the findButton method of the processManager. And maybe putting a warning in the next release note and maybe a change record will be good.
Comment #5
markhalliwellDon’t remove the method in
ProcessManager, just deprecate it andreturn $element->findButton();.Also, above that, call
Bootstrap::deprecated().Comment #6
grimreaperOK... Now I see how you handle deprecation.
Here is a new patch.
Thanks for the review.
Comment #8
markhalliwellComment #9
grimreaperThanks for the commit :)