I spotted this problem adding a site search form that refused to display inline. Before creating a subtheme component with lots of custom CSS, I found the documentation that says that moving from Bootstrap 4 to 5:

Breaking Dropped form-specific layout classes for our grid system. Use our grid and utilities instead of .form-group, .form-row, or .form-inline.

The solution for my search form block was to replace form-inline in the <form> class list with d-flex flex-row flex-wrap.

I'll see, later, if I can turn this into a patch / merge-request. Although ideally someone who knows this base theme better than I do (and who probably knows the Bootstrap framework better than me too) would do this, because this is a theme with lots of components. Proposing something that fixes the bits of it used by my sites does not mean this is resolved for everyone, and I might miss things. So for now I'll leave this here.

Issue fork radix-3507333

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:

About issue forks

Comments

jamesoakley created an issue. See original summary.

doxigo’s picture

doxigo commented

Hey James, Thanks for a nice find, please provide the MR, I appreciate it, thanks

jamesoakley opened merge request !132

jamesoakley’s picture

jamesoakley
Primary language English
Location North Yorkshire, UK
commented
Status: Active » Needs review

So, I think that merge request is the way to do it.

It assumes that the bootstrap classes I've added in place of form-inline are the correct ones. I'd be glad for someone to check that. (It works on the site I'm testing on, at all browser widths, but I'm not 100% sure this is the canonical Bootstrap way to do this).

Doing it in the Form component means that any templates that call radix:form with is_inline:true will pick up the new markup without having to change every templates. So I think is the DRY way to fix this.

doxigo’s picture

doxigo commented

Thanks James, I'm not sure if flex-row and flex-wrap are needed, are they? can you test without? if so update the MR?

jamesoakley’s picture

jamesoakley
Primary language English
Location North Yorkshire, UK
commented
Status: Needs review » Needs work

Also just triggered an error on another pageload, so I'm just investigating that.

jamesoakley’s picture

jamesoakley
Primary language English
Location North Yorkshire, UK
commented
Status: Needs work » Needs review

OK

My twig bug was because of a hastily (and embarrassingly) mistyped ternary operator. Oops.

I can confirm that you don't need flex-row, because that's the default

It seems that you don't need flex-wrap either, but I'm testing on the search form which is pretty small. The default is flex-nowrap, and I'm not clear what happens if it's a bigger inline form. I can put flex-wrap back if that's desirable, but for the time-being the latest iteration of the MR works fine for me.

doxigo’s picture

doxigo commented
Status: Needs review » Fixed

Perfect work James, merged

danchadwick’s picture

danchadwick commented
Status: Fixed » Active

Eh, this is a breaking change. I use form-inline for a number of custom styles. I don't think we can just remove that class without breaking sites.

Suggest we add to form-inline, rather than replace it. @doxigo: thoughts?

danchadwick’s picture

danchadwick commented
Status: Active » Needs work

jamesoakley opened merge request !133

jamesoakley closed merge request !133

jamesoakley opened merge request !134

jamesoakley’s picture

jamesoakley
Primary language English
Location North Yorkshire, UK
commented
Status: Needs work » Needs review

I'd be inclined to agree.

form-inline won't get picked up by Bootstrap to do anything.

But you're quite right - that doesn't mean people won't be picking up on the class to do their own CSS.

Having got into a tangle with how Gitlab's merge requests work when you making further changes to the same issue fork, I think the latest MR is now what's needed. Tested and it still works, but also leaves form-inline in place.

doxigo’s picture

doxigo commented

Hey Dan, you are correct, I missed the breaking change part, it definitely is. We should ideally keep it and also add the new d-flex class.

Thanks for the updated MR James

doxigo’s picture

doxigo commented
Status: Needs review » Fixed

The MRs were a bit confusing, I just kept both the form-inline and d-flex

  • doxigo committed d6260d59 on 6.0.x 
    Issue #3507333 by jamesoakley, doxigo, danchadwick: Bootstrap 5 removed...
jamesoakley’s picture

jamesoakley
Primary language English
Location North Yorkshire, UK
commented

Sorry for the confusion - sometimes MR aren't the easiest way to put a change in. Yes, keeping both is correct - one to ensure it works with Bootstrap 5, one to avoid breaking any sites that have built CSS off the old classes. Thanks for fixing

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.