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.In #2675464: Margin on form-items in table rows (Classy) we identified and devised a solution for a problem in the Classy theme that causes problems for modules like IEF and Paragraphs. However, @lauriii suggested it was too dangerous to fix properly in Classy without trying first in Seven.
Problem
In /core/themes/classy/css/components/form.css we have:
tr.odd .form-item,
tr.even .form-item {
margin-top: 0;
margin-bottom: 0;
}
This works fine for single element forms (like checkboxes or selects) in table rows, but causes problems when we want to have more complex multi-element forms in tables (like anywhere in a standard node edit form when a field widget requires multiple form elements).
Furthermore, this may break break content list styles; see comment 47 in #2675464-47: Margin on form-items in table rows (Classy).
Solution
Add the margin back in, overriding Classy. Then remove it just for td > .form-item:only-child.
Todo
Create a patchPost screenshots showing before/after in a complex form like paragraphs- Post screenshots showing before/after in the content list
Tag for Needs frontend framework manager review- Copy issue credit across from #2675464: Margin on form-items in table rows (Classy)
Screenshots
Before:
After:
| Comment | File | Size | Author |
|---|---|---|---|
| #30 | interdiff.txt | 423 bytes | lauriii |
| #24 | after.png | 54.47 KB | anmolgoyal74 |
| #24 | before.png | 39.71 KB | anmolgoyal74 |
| #24 | 2982097-23.patch | 722 bytes | anmolgoyal74 |
Comments
Comment #2
jonathanshawComment #3
Anatoliy Vorobyov CreditAttribution: Anatoliy Vorobyov at FFW commentedPatch added.
Comment #4
jonathanshawthanks @Anatoliy
Comment #5
lauriiiMoving to needs review to encourage contributors to take screenshots 🙌
Comment #6
lauriiiThis still changes Classy. We should implement this so that it only affects Seven. Leaving to needs review for the manual testing since knowing if something is broken would be useful already at this point.
Comment #7
jonathanshawSorry @lauriii I forgot to make the most important thing clear in the IS!
Comment #8
KondratievaS CreditAttribution: KondratievaS at Skilld commentedI have tested and patch works as expected - margins are added
Comment #9
jonathanshawThanks @Kondratieva. Putting to Needs Work temporarily in the hope that will attract someone to fix #6
Comment #10
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedI have tried testing it in 'seven' theme with some changes in the patch #3.
Comment #11
jonathanshaw@anmolgoyal74 it would help if you could explain the reason for changing the margin to 0.75em instead of the 0em suggested in the IS based on the parent issue.
Comment #12
jhcastanod CreditAttribution: jhcastanod as a volunteer and commentedI tested the solution provided and worked as expected
Comment #13
lauriiiThe logic here doesn't seem right. We should make sure that there's no margin when
.form-itemelements are placed as only child directly inside atdelement. When that isn't the case, the default margin given for.form-itemshould apply.Comment #14
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedwe cannot have margin set to 0 only to element that are placed as only child directly inside a td element without removing the following CSS in Classy theme.
Or we can instead add margin to other cases as in #10.
Else we can proceed with patch in #3
Comment #15
jonathanshawIn #2675464: Margin on form-items in table rows (Classy) we ended up with:
Maybe that would work here too.
Comment #16
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commented#15 doesn't work till we have the CSS in classy which needs to be removed to reflect the changes.
If we remove this from classy then #15 works well.
This code sets margin to 0 for every element. So we need to add margin to other cases.
Comment #17
jonathanshawGood point. But I'm still not understanding #10. Shouldn't we be doing something like this in seven to first reverse classy then reapply in a more targeted fashion:
Comment #18
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedNice workaround.
But #17 isn't working.
This is working
Comment #19
jonathanshawGreat. For this to be RTBC we're going to need a code comment. Perhaps something like:
* When a table row has a single form item, prevent it from adding unnecessary
* extra spacing. If it has multiple form items, allow spacing between them,
* overriding Classy.
Comment #20
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedComment #21
jonathanshawLet's see what @lauriii says
Comment #23
lauriiiCould this be changed to be only
td > .form-item:only-childor isn't that heavy enough to override the other selector? Besides that this starts to looks good for me.Comment #24
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedComment #25
jonathanshawI can't recreate the content list problem that @lauriii reported in #2675464-47: Margin on form-items in table rows (Classy), so can't provide a screenshot to show it's fixed.
Comment #26
jonathanshawComment #30
lauriiiCredited people from #2675464: Margin on form-items in table rows (Classy) to make sure their work gets credited. I also made a minor code style adjustment on the commit (interdiff attached).
Committed 25809ca and pushed to 8.7.x. Thanks!
Comment #32
lauriiiCherry-picked f139cc7 and pushed to 8.6.x.