Buttons detected in .form-actions are copied to the modal's footer region and the originals are hidden. When those are clicked, it clicks the real one it was cloned from in the form. Unfortunately, if there's an AJAX update, this clone binding gets broken (because it's a new button).
When embedding an image with Entity Embed, and end up on the thumbnail options dialog, there are no submit buttons at the bottom of the dialog. Sometimes there is a hidden button that has focus.
When adding an image to an article or basic page via the body field wysiwyg editor, failing to enter alt-text means you can't save the image.
Set bootstrap as the admin theme. Then:
1. Go to /node/add/article
2. Click the image icon in the editor
3. Upload an image
4. DON'T add alt text, then click save.
5. View error saying alt text is required.
6. Add alt text, click save.
7. Nothing happens.
If you add alt text to begin with, it works fine.
Patch at #4 manually updates the "fake modal buttons" by copying elements to the footer.
However, This JS should probably hide any existing form action buttons, clone them and then inject those clones into the footer then proxy all events to the original form action element (like is now currently done in ./js/dropdown.js). This will ensure that this works with all contrib modules.
GIF showing manual test Before and After patch #4.
* Issue summary update.
* Another review or 2?
* Tests?
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | remove_modal_footer_btn-2838190-17.patch | 2.16 KB | julienvey |
| #5 | 2531700-manualtest-5.gif | 997.47 KB | kattekrab |
| #5 | 2838190-codereview-5.png | 66.4 KB | kattekrab |
| #4 | Screen_Shot_2017-02-27_at_2_22_12_PM.jpg | 58.25 KB | milesw |
Comments
Comment #2
markhalliwellI'm honestly unsure this would be a Bootstrap specific issue (at this point). The base theme doesn't alter that form (specifically) and any issue would likely be on the back-end (with core). That being said, it appears that got fixed?
It could possibly be a modal/dialog issue... but that seems rather unlikely. Could try the patch here: #2826277: Rework/update dialog JavaScripts
Comment #3
markhalliwellSo... this is, in fact, a bug with this base theme. Has to do with the dialog/modal JS crap that needs work on.
Basically, what's happening is that any buttons detected in
.form-actionsare copied to the modal's footer region and the originals hidden.When those are clicked, it clicks the real one it was cloned from in the form.
Unfortunately, if there's an AJAX update, this clone binding gets broken (because it's a new button).
I'll keep this issue open because it's a very specific problem.
Comment #4
milesw commentedRan into this with Entity Embed. When you go to embed an image, and end up on the thumbnail options dialog, there are no submit buttons at the bottom of the dialog. Sometimes there is a hidden button that has focus. See attached screenshot.
The problem is that the method being used to update the buttons
$dialog.modal('option', 'buttons', buttons);isn't doing anything.Here is a patch that manually updates these "fake modal buttons" by copying elements to the footer.
Comment #5
kattekrab commentedOh awesome. Thanks @milesw
I've just done a side by side manual test using simplytest.me - this patch works!
I'm not a dev, but have had a quick look at the code itself. It seems straightforward, no obvious problems, but I'm not qualified to say for sure. I do wonder though if we need tests for this?
Screengrab of the patch via dreditor, and a gif of manual test below. I'd say RTBC, but probably need a dev to actually review the code, and @markcarver to say whether this needs tests.
edit: removed gif, added to IS.
Comment #6
markhalliwellI'd much rather we not create arbitrary buttons.
This JS should probably hide any existing form action buttons, clone them and then inject those clones into the footer then proxy all events to the original form action element (like is now currently done in
./js/dropdown.js). This will ensure that this works with all contrib modules.Comment #7
kattekrab commentedComment #8
kattekrab commentedIS tidy
Comment #9
kattekrab commentedComment #10
alx_benjamin commentedCan this patch be deployed?
Comment #11
markhalliwellSee #6.
Comment #12
markhalliwellComment #13
adrianavaz commentedThis patch removes the fake modal buttons from modal-footer.
Comment #14
adrianavaz commentedThis patch removes the fake modal buttons from modal-footer.
Comment #15
agolubic commentedCurrent patch is not working after bootstrap update to 3.16.0. New patch in attachment :)
Comment #16
markhalliwellClearly, something isn't working... but removing them entirely isn't really a proper "fix".
Comment #17
julienvey commentedNew patch for bootstrap 3.23.0 :)
Comment #18
julienvey commented