Closed (fixed)
Project:
Bootstrap
Version:
8.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
14 Feb 2018 at 15:30 UTC
Updated:
6 Mar 2018 at 17:24 UTC
Jump to comment: Most recent, Most recent file
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 :)