Hi,
Here is another couple of lines I always need to undo in my child themes. Is there a reason for the base theme to add the class "form-inline" to all elements of a certain type? It makes the forms elements display in an inconsistant manner.
Maybe this behaviour could be opted in with a setting in the appearance form?

@@ -66,9 +66,6 @@ public static function process(array $element, FormStateInterface $form_state, a
     if ($e->hasClass('container-inline')) {
       $e->replaceClass('container-inline', 'form-inline');
     }
-    if ($e->isType(['color', 'date', 'number', 'range', 'tel', 'weight'])) {
-      $e->addClass('form-inline', 'wrapper_attributes');
-    }
 
     // Process input groups.
     if ($e->getProperty('input') && ($e->getProperty('input_group') || $e->getProperty('input_group_button'))) {

Comments

joos created an issue. See original summary.

markhalliwell’s picture

Title: Remove form-inline from numeric fields » Allow overriding of elements that are automatically converted to "form-inline"
Priority: Minor » Normal

This is, inherently, an opinionated topic.

These elements were designed to be inline because their inputs usually only require small amounts of data.

Also, simply removing this would break other people sites that either a) already like that this is implemented for them automatically or b) implemented workarounds to deal with this.

Instead of removing this completely, I would say that this plugin should implement a property on the class that can be overridden by a sub-theme as needed. This would allow developers to either a) remove the elements completely (as show above) or b) extend it with additional elements.

An example of how this has already been accomplished in the ThemeSuggestions alter plugin can be seen here:

https://www.drupal.org/node/2838155#after
https://drupal-bootstrap.org/api/bootstrap/src%21Plugin%21Alter%21ThemeS...
https://drupal-bootstrap.org/api/bootstrap/src%21Plugin%21Alter%21ThemeS...

joos’s picture

Hi Mark,
Yea, what to do and not to do is probably gonna be a popular topic i theme-project like this. I'm with the do-less side of that debate. :)

I know, we can't change that behaviour, thats why I suggested a setting for that so one could get a consistant behaviour, but the out-of-the box version still would function like it do today. Im not personally found of making code that undo stuff later in the execution chain since that is only gonna make sites slower. But what you suggest is maybe not like that at all. Gonna look at the links later.

Thanks for your reply.

JordiK’s picture

Hi Mark,

agree with you, that the 5 fields listed will most of the time require a narrower field and a small amount of data.
But the placement of the label is always inline, which makes the forms look weird, when all other field types have the label above.
What would be the preferred way to set the label position of a field (UI or a property or ...)?

Thanks in advance!

markhalliwell’s picture

But the placement of the label is always inline, which makes the forms look weird, when all other field types have the label above.

Hm. I hadn't thought about that. Yeah, that is a little off-putting.

What would be the preferred way to set the label position of a field

Probably via CSS.

---

The main problem is that Bootstrap (the framework) does not provide any way to natively reduce a form element's width. They're all 100% width, unless inline. This, as you pointed out, also comes with it's own assumptions applied to the label.

I think, perhaps, we'll need to add a special class in this base theme that, when applied, will just reset only the input element's width back to the natural auto setting.

joos’s picture

But the placement of the label is always inline, which makes the forms look weird, when all other field types have the label above.

Exactly what I ment with "inconsistant behaviour". Thanks for putting it in plain english!

My solution sofar is to revert the form-inline styles with a more specific css rule in my sass files. But I just tried an alternative, I made a copy of the template "form-element.html.twig" in my base theme and added ".removeClass('form-inline')" at row #73.

<div{{ attributes.addClass(classes).removeClass('form-inline') }}>

Looks promising and could perhaps also be an option on how to customize forms in other bootstrap ways, inline, horizontal etc.

markhalliwell’s picture

Status: Active » Postponed

This actually cannot happen right now since the ProcessManager cannot be sub-classed in a sub-theme.

chrisolof’s picture

Would a theme configuration setting be acceptable here?

gagarine’s picture

As #2829634: Why is telephone field inline? was closed, let me copy past here what I think about this bug.

This feature cause unattended consequences in different module like webform #2910776: Telephone Field, Title display Before same as Inline.

Their is already settings to manage label alignment in Drupal. Making this hard coded in the bootstrap theme break those settings.

Please remove it. If it break old website it's ok, it's not going to make the website unusable. Give a warning in the log and a bit of code that peoples can simply drop in their sub theme to keep the old behavior if really they want that (I doubt they do want).

These elements were designed to be inline because their inputs usually only require small amounts of data.

Yet it's know that labels is habitually better on top of the field. And it should (almost) never be mixed.

From https://uxplanet.org/designing-more-efficient-forms-structure-inputs-lab...
"If you want users to scan out a form fast, put your labels above each field."

tamarpe’s picture

It should be at least a theme configuration IMO, Hardcoded in the bootstrap theme break the settings of the Drupal to manage label alignment.. Can it be active again?

gagarine’s picture

@tamarpe I end up making my own bootstrap 4 starter theme: https://github.com/gagarine/bootstrap4

I will never look back.

Chris Matthews’s picture

Ran into this issue today. Should the issue status still be postponed, or should it be set to active?

markhalliwell’s picture

Version: 8.x-3.x-dev » 8.x-4.x-dev

This issue, specifically, is postponed due to #7.

I've thought about this a bit. I think a new issue can be created that introduces a new method on that class (maybe something like getInlineElementTypes). That method can provide a default list and then invoke a theme alter so people who want/need to change that list can. It, of course, should be statically cached in that method as well since it will be invoked a lot and we just need to get the value once so it can be used in place of what's currently there.

edit: this new method should also be marked as internal and deprecated, linking to this issue since it will be removed once this issue can be resolved

markhalliwell’s picture

shelane’s picture

Status: Postponed » Closed (won't fix)

This theme will not be supported for Bootstrap 4. See alternative themes for this support.