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.
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
findButton
method really doesn't belong inProcessManager
anyway and should instead live in theElement
class instead.I think this patch should move the method (getting rid of the unnecessary
$element
parameter) and then deprecate this existing method?Then it should be a simple as
$button = &$parent->findButton()
inprocessInputGroups
Comment #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 :)