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.
Problem/Motivation
Currently the actions button/dropbutton on the end of a node add/edit form only shows "Save and publish". But there is another button "Save as unpublished". You can see those elements in the source code, but the element is not correctly rendered as split button (I guess it should be one). See the attached screenshot and compare with Bootstrap's component example.
Proposed resolution
- Fix rendering of actions button/dropbutton as Bootstrap split button
Remaining tasks
- The dropdown toggle button has to receive the same style variant class as the button itself has. Otherwise, the dropdown toggle's colors won't match the rest of the split button.
- Recompile and commit CSS before merging
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#18 | interdiff.txt | 6.4 KB | markhalliwell |
#18 | dropbutton_actions-2824693-18.patch | 13.62 KB | markhalliwell |
Comments
Comment #2
hctomComment #3
markhalliwellI believe this may have broke when attempting to fix this related issue :-/
Comment #4
hctomThe attached patch should fix the issue. You expected the
$variables->links
array to have more than one item, but with$variables->toggle = array_shift($variables->links);
(line 66) you already remove the first item from the array for the button itself.Apart from that, the patch also contains LESS/SASS changes to discard the
margin-right: 10px;
from buttons in button groups. Otherwise there is some space between the toggle and the button itself.NOTE: The patch only contains the changes in the source files. I did not add all changed CSS files, because it bloats the patch up to 4.4 MB (as EVERY minified CSS file changes due to the fixes/customizations)
... and there is still one thing left to fix (perhaps someone else has a quick solution for that):
The dropdown toggle button has to receive the same style variant class as the button itself has. Otherwise, the dropdown toggle's colors won't match the rest of the split button.
Comment #5
hctomOooops... found a little mistake in my changes. Here is an updated patch.
Comment #6
hctomComment #7
markhalliwellYes, just changing the source files is perfect. The CSS is automatically compiled using the grunt compile command when a maintainer commits changes to the source files. I opened #2824750: Document how to contribute CSS changes to help remedy this confusion.
Ah, this makes sense.
Well, if that's the case, I'd say let's just booleanize based on whether the array is empty or not instead of invoking
count()
:Since there's already a
.form-actions
above, let's just add this to that ruleset:Comment #8
markhalliwellThis should already be handled by Element::colorize, Element::setButtonSize and InputButton::preprocessElement.
Comment #9
hctomThanks for the feedback. Attached you can find the updated patch. And thanks for your method links - those guided me to the root of the color problem ;) The color variant class was not set, if colorizing is disabled - but that is fixed with this patch, too.
Comment #10
hctomLet's remove that duplicate check of
$button
... damn copy & paste ;)Comment #11
hctomComment #12
hctomOkay... but now this is really the last updated patch ;)
Just found duplicate setting of toggle button size, so I removed the statement from the template - size is already set in setButtonSize() for toggle button
Comment #13
markhalliwellYep, this works now.
Decided to refactor the
colorize
andsetButtonSize
methods so we're not having to duplicate the split button logic.Tested it myself and it looks fine, but just want to get an OK from you before committing.
Comment #14
hctomI will have a look at it, but first: Here is an interdiff to be able to track the changes more easily ;)
Comment #15
hctomDamn... forgot the upload ;)
Comment #16
hctomAnd let's reorder the files again... don't know what happened there
Comment #17
hctomSo... just got time to have a look at the refactoring. This looks fine on the
colorize()
method to me, but there is one problem in thesetButtonSize()
:You have this array with all sizes:
The problem is, that
.btn-*
may be combined with.btn-block
(e.g. to have a large button that spans the full width). With your implementation, setting the size would clear the block variant class or vice versa when trying to add the block variant, the size would be deleted.Comment #18
markhalliwellAh, yes... IDK why
.btn-block
is even in there. I refactored this some more so it pulls the active theme'sbutton_size
option keys instead. That way, if it ever changes in the settings, it'll be reflected there too.Comment #19
hctomSo... finally got the time time check your refactoring. Looks very good and definitely makes sense to avoid duplicated option definitions. The only thing I found, is one cod style issue (at
Theme::getSettingPlugin()
):@return \Drupal\bootstrap\Plugin\Setting\SettingInterface|\Drupal\bootstrap\Plugin\Setting\SettingInterface[]|NULL
NULL
should benull
, as core uses it in its sources.I saw there are some more places where
NULL
is used in docblocks, so what do you think? I'd change them all tonull
, even though this is really nitpicky ;)Feel free to ignore this, so we get the actual dropbutton fix commited - that's why I set it to RTBC for now.
Comment #20
markhalliwellYes, let's commit as is then. The data type can be addressed per this related issue.
Comment #22
markhalliwellComment #24
markhalliwell@hctom, please see #2838956: Events bound to original dropdown elements are lost