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.
Olivero's currently stacks Views exposed filters. Other themes inline them. We should do the same.
Comment | File | Size | Author |
---|---|---|---|
#11 | Screenshot_8_17_21__9_51_AM.png | 53.85 KB | mherchel |
#11 | interdiff-5-11.txt | 1.11 KB | mherchel |
#11 | 3224958-11.patch | 1.74 KB | mherchel |
#9 | Screen Shot 2021-08-17 at 15.40.36.png | 15.41 KB | lauriii |
#6 | 3224958-after.png | 15.73 KB | cindytwilliams |
Comments
Comment #2
djsagar CreditAttribution: djsagar at OpenSense Labs commentedComment #3
djsagar CreditAttribution: djsagar at OpenSense Labs commentedHi @mherchel,
I create patch for Views exposed filters to making inline.
Please review.
Thanks!
Comment #4
djsagar CreditAttribution: djsagar at OpenSense Labs commentedComment #5
mherchelI like the idea of adding specific
.form--inline
styles. I wasn't initially considering it, but it makes sense.I'm attaching an updated patch that adds that class directly onto the Views exposed filters form, and I'm also using
display: inline-block
to handle the inline elements instead offloat
. This will handle RTL a bit better, and is more what we're trying to do.Comment #6
cindytwilliams CreditAttribution: cindytwilliams at Kanopi Studios commentedPath #5 applies cleanly, and displays the Views exposed filters inline using the form--inline class. Marking RTBC.
Before:
After:
Comment #8
mherchelLast test failure is unrelated.
Comment #9
lauriiiHow do we expect this to look when the form element has a description?
Comment #10
mherchelNot like that!
Comment #11
mherchelFix attached!
Comment #12
andy-blumThis all looks good! Moving to RTBC.
Comment #14
mherchelTest failure is unrelated.
Comment #16
mherchelAnother unrelated Layout Builder test failure:
Comment #17
mherchelComment #18
mherchelOpened followup #3232665: 'forms-inline' class should ensure all direct descendants are inline
Comment #20
lauriiiTested this manually in various use cases and it worked as expected!
Something that caught my attention was that
form--inline
isn't implementing BEM properly. However, it seems to be a pre-existing issue. Discussed with @mherchel and we agreed that this is something that should be handled globally, for example in #2417111: Replace container-inline with form--inline to display forms horizontally..Our pre-existing use cases are using
.form--inline .form-item
as a selector. I like the approach here because I think it's fair to assume that inside.form--inline
all elements are inline, regardless of whether they have.form-item
class or not. The approach on this patch also seems potentially more compatible with BEM because the approach currently taken by core, adds a dependency between block level elements. Discussed with @mherchel and he said he'd open a follow-up for refactoring other themes to use this pattern.Committed 8c35867 and pushed to 9.3.x. Thanks!
Didn't backport to 9.2.x because of the new hook implementation.