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.

Issue fork drupal-3232665

Command icon 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:

Comments

mherchel created an issue. See original summary.

pragati_kanade’s picture

Assigned: Unassigned » pragati_kanade

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

elber’s picture

Hi @pragati_kanade are you still working on this?

elber’s picture

Assigned: pragati_kanade » elber
elber’s picture

I keep working on it

elber’s picture

Assigned: elber » Unassigned
Status: Active » Needs review

Hi @mherchel please revise I don't know if I understood your comment in Slack.

mherchel’s picture

Status: Needs review » Needs work

This 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

elber’s picture

Assigned: Unassigned » elber

Sorry, I will try to work again.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

elber’s picture

Assigned: elber » Unassigned
arunkumark’s picture

Status: Needs work » Needs review
StatusFileSize
new54.85 KB

Seems 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/olivero

CSS is available on the Olivero theme.
Code exist

Note: reopen the issue if the issue persists.

ranjith_kumar_k_u’s picture

StatusFileSize
new49.37 KB
new544 bytes
ranjith_kumar_k_u’s picture

StatusFileSize
new51.86 KB
new16.07 KB

Just complied media-library.pcss.css file

anjali rathod’s picture

The patch looks good to me and applies correctly.

mherchel’s picture

Status: Needs review » Needs work

The patch adds a new CSS file /core/themes/olivero/css/theme/media-library.css

Is this intentional? If so, what's the reasoning? It seems unrelated.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bnjmnm’s picture

Issue tags: +Field UX

Tagging 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.

rohan-sinha’s picture

StatusFileSize
new83.69 KB

The CSS codes are available in the repository.

grep -r 'form--inline' core/themes/olivero

gauravvvv’s picture

Status: Needs work » Needs review
StatusFileSize
new51.45 KB
new445 bytes

Addressed #17, attached interdiff for same. please review

smustgrave’s picture

Status: Needs review » Needs work

Why are we adding a css folder to core?

/core/css/form--inline.css

gauravvvv’s picture

Status: Needs work » Needs review
StatusFileSize
new1.84 KB
new2.28 KB

I 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.

rohan-sinha’s picture

the issue, summary say Olivero theme but the above patch is for claro, demo_umami and starterkit_theme, looks summary needs to be updated:
Suggestion:

The 'forms-inline' class should ensure all direct descendants are inline within all the core themes provided namely: claro, olivero, demo_umami and starter kit

aziza_a’s picture

Checked the patch given on #23 apply cleanly and looks good to proceed

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Could the issue summary be updated for what the target themes are please

smustgrave’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs issue summary update +Needs Review Queue Initiative

Reread it and the issue summary mentions we should be done in all of core and since those are the themes should be fine.

lauriii’s picture

Status: Reviewed & tested by the community » Needs work
index b5201a78c9..a230e0982f 100644
--- a/core/profiles/demo_umami/themes/umami/css/classy/components/inline-form.css

--- a/core/profiles/demo_umami/themes/umami/css/classy/components/inline-form.css
+++ b/core/profiles/demo_umami/themes/umami/css/classy/components/inline-form.css

index b5201a78c9..e762ff9682 100644
--- a/core/themes/claro/css/classy/components/inline-form.css

--- a/core/themes/claro/css/classy/components/inline-form.css
+++ b/core/themes/claro/css/classy/components/inline-form.css

At the moment these CSS files are in the classy directory 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 the classy directory.

narendrar’s picture

StatusFileSize
new2.5 KB
new806 bytes

Addressed #28

bnjmnm’s picture

+++ b/core/themes/claro/claro.libraries.yml
@@ -40,6 +40,7 @@ global-styling:
       css/components/form--select.css: {}
       css/components/help.css: {}
       css/components/image-preview.css: {}
+      css/components/inline-form.css: {}
       css/components/menus-and-lists.css: {}
       css/components/modules-page.css: {}
       css/components/node.css: {}

This addition looks good, but the library definition should also have its reference to css/classy/components/inline-form.css: {}
removed

narendrar’s picture

Status: Needs work » Needs review
StatusFileSize
new2.84 KB
new569 bytes

Addressed #30

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Since we are moving files can we update the issue summary please.

narendrar’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update

The last submitted patch, 29: 3232665-29.patch, failed testing. View results

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the IS update.

The last submitted patch, 29: 3232665-29.patch, failed testing. View results

The last submitted patch, 29: 3232665-29.patch, failed testing. View results

bnjmnm’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new457.9 KB
new266.82 KB

I'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.

ravi kant’s picture

Assigned: Unassigned » ravi kant
ravi kant’s picture

Assigned: ravi kant » Unassigned
Status: Needs work » Needs review
StatusFileSize
new113.04 KB
new730 bytes

The 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.

bnjmnm’s picture

Status: Needs review » Needs work

Probably 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.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bnjmnm’s picture

Issue tags: +Needs issue summary update, +Pittsburgh 2023

Currently 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.

bnjmnm’s picture

Issue tags: -Pittsburgh 2023 +Pittsburgh2023

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.