This is a followup from #3224958: Olivero should style Views exposed filters as inline (currently are stacked)/
Within Olivero we're adding the following code which ensures that all direct descendants will be inline (which is the expectation when setting that CSS class)
.form--inline > * {
display: inline-block;
margin-top: 0.5625rem;
margin-bottom: 0;
vertical-align: top; /* Ensure proper alignment if description is present. */
}We should do this within all of Drupal core, as it's a more robust solution rather than relying on the child elements CSS class.
The file /core/themes/claro/css/classy/components/inline-form.css is currently located in the classy directory because it is an exact copy of the Classy theme. However, since the file will be modified, it is recommended to move it outside of the classy directory.
| Comment | File | Size | Author |
|---|---|---|---|
| #38 | reuse-after.png | 266.82 KB | bnjmnm |
| #38 | iu-before-after.png | 457.9 KB | bnjmnm |
| #31 | interdiff-29_31.txt | 569 bytes | narendrar |
| #31 | 3232665-31.patch | 2.84 KB | narendrar |
| #29 | interdiff-23_29.txt | 806 bytes | narendrar |
Issue fork drupal-3232665
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3232665-forms-inline-class-should
changes, plain diff MR !2035
Comments
Comment #2
pragati_kanade commentedComment #4
elberHi @pragati_kanade are you still working on this?
Comment #5
elberComment #6
elberI keep working on it
Comment #8
elberHi @mherchel please revise I don't know if I understood your comment in Slack.
Comment #9
mherchelThis MR looks to be unrelated. It looks like it's adding media library CSS?
Anyway, we don't need to do that. The goal is to add some CSS into Drupal core (outside of themes) that ensures all direct descendants of 'forms-inline' are inline. You can look to the Olivero CSS for reference, but this task doesn't involve adding anything to Media or Olivero
Comment #10
elberSorry, I will try to work again.
Comment #12
elberComment #13
arunkumarkSeems the patch is not required for the 9.5.x version. The CSS codes are available in the repository.
grep -r 'form--inline' core/themes/oliveroCSS is available on the Olivero theme.

Note: reopen the issue if the issue persists.
Comment #14
ranjith_kumar_k_u commentedComment #15
ranjith_kumar_k_u commentedJust complied
media-library.pcss.cssfileComment #16
anjali rathodThe patch looks good to me and applies correctly.
Comment #17
mherchelThe patch adds a new CSS file
/core/themes/olivero/css/theme/media-library.cssIs this intentional? If so, what's the reasoning? It seems unrelated.
Comment #19
bnjmnmTagging Field UX as there's a good chance of future developments making greater use of inline form elements.
If there are backwards compatibility concerns with the solution as proposed that don't seem sufficiently addressed by stable, another option is to create a new class that has the proposed changes targeting direct descendants, but keep the current one working as is.
Comment #20
rohan-sinha commentedThe CSS codes are available in the repository.
grep -r 'form--inline' core/themes/olivero
Comment #21
gauravvvv commentedAddressed #17, attached interdiff for same. please review
Comment #22
smustgrave commentedWhy are we adding a css folder to core?
/core/css/form--inline.css
Comment #23
gauravvvv commentedI have updated the code in all themes instead of creating a single directory inside the core, as in patch #21. I have updated the patch and attached an interdiff for the same.
Comment #24
rohan-sinha commentedthe issue, summary say Olivero theme but the above patch is for claro, demo_umami and starterkit_theme, looks summary needs to be updated:
Suggestion:
Comment #25
aziza_a commentedChecked the patch given on #23 apply cleanly and looks good to proceed
Comment #26
smustgrave commentedCould the issue summary be updated for what the target themes are please
Comment #27
smustgrave commentedReread it and the issue summary mentions we should be done in all of core and since those are the themes should be fine.
Comment #28
lauriiiAt the moment these CSS files are in the
classydirectory because they are exact copies of Classy. However, not that we are modifying these, it is no longer the case. For that reason, we should move them outside theclassydirectory.Comment #29
narendrarAddressed #28
Comment #30
bnjmnmThis addition looks good, but the library definition should also have its reference to
css/classy/components/inline-form.css: {}removed
Comment #31
narendrarAddressed #30
Comment #32
smustgrave commentedSince we are moving files can we update the issue summary please.
Comment #33
narendrarComment #35
smustgrave commentedThanks for the IS update.
Comment #38
bnjmnmI'm seeing some regressions with this change. It's probably worth checking all usages of .form--inline and making sure they continue to look OK.
Comment #39
ravi kant commentedComment #40
ravi kant commentedThe issue appears when the "Exposed form" is displayed in a block. During the display "Exposed form" in the block, the form--inline class adds on outer div instead form tag. so fixed the issue according to it and created a patch.
Comment #41
bnjmnmProbably best to disregard #40, but hopefully the feedback here helps you give it another shot @ravi kant
The issues reported in #38 are happening in the Claro theme
The change in #40 is being made to the Olivero theme. This would have no impact whatsoever.
In addition, the changes were made to form.css. Olivero and Claro changes need to be made to .pcss.css files, then the build process creates the css files.
Nothing in #38 deals with views exposed forms so that's probably not the place to target. Also, screenshots of html isn't particularly helpful, but seeing how it appears as an actual styled page can be helpful.
Comment #43
bnjmnmCurrently the issue summary explains this change should happen Olivero is doing it. #38 points out there are regressions. The issue summary should be updated to better explain why this is a beneficial change. If that is documented, we can better decide if making the actual potential-regression-causing change is worthwhile.
Comment #44
bnjmnm