Improve form BS support

For now last things that needs to be done before closing this issue are

• Required form elements marker is missing, and while there check error validations styles also, see can we use for that https://v4-alpha.getbootstrap.com/components/forms/#validation-states.
• Group of radio or checkbox buttons is not rendered correctly - missing some BS classes there, see https://v4-alpha.getbootstrap.com/components/forms/#form-controls.
• Try to implement file browsers component, see https://v4-alpha.getbootstrap.com/components/forms/#file-browser.

1. +++ b/themes/bs_bootstrap/sass/components/forms.scss
   @@ -3,9 +3,66 @@
   +    color:red;
   

   This should be a variable and not hard coded color value - check for BS danger/warning or some appropriate color value that already exist.

2. +++ b/themes/bs_bootstrap/sass/components/forms.scss
   @@ -3,9 +3,66 @@
   +.form-file {
   ...
   +.custom-file-control {
   

   Why are we having here custom code? Why are we not including bootstrap/sass/_custom_forms.scss?

   Also instead of .form-file class why we are not using BS .custom-file-input which looks that is doing same thing?

   Also lots of hardcoded variables - even with custom code we should avoid hard values but use our custom variables. Not relevant in this case if we are going to use bootstrap sass file for this.

3. +++ b/themes/bs_bootstrap/templates/form/input--file.html.twig
   @@ -0,0 +1,10 @@
   \ No newline at end of file
   

   Missing new line.

While testing this i found additional related problems, created two follow ups #2878380: Improve inline form error support, #2878378: Visual look of multi file field is not good.

1. +++ b/themes/bs_bootstrap/css/components/forms.css
   @@ -265,13 +265,222 @@ select.form-control-lg:not([size]):not([multiple]) {
   +.custom-file-control:lang(en)::after {
   +  content: attr(data-content);
   +}
   +
   +.custom-file-control:lang(en)::before {
   +  content: attr(data-browse);
   

   Okey lang(en)?

   Why do we have language dependent selector? We will build lots of multilingual sites so this will not work like this.

2. +++ b/themes/bs_bootstrap/sass/components/forms.scss
   @@ -1,11 +1,26 @@
   +  @each $lang, $text in map-get($custom-file-text, placeholder) {
   +    &:lang(#{$lang})::after {
   +      content: attr(data-content);
   +    }
   +  }
   

   Ah i see i guess per each language you offer different content.

   But i think we do not need this, we can provide translatable data-content attribute in twig and translate there the string so we do not need selector per language i guess, right?

3. +++ b/themes/bs_bootstrap/templates/form/input--file.html.twig
   @@ -0,0 +1,10 @@
   +  <span data-content="{{ 'Choose a file...'|t }}" data-browse="{{ 'Browse'|t }}" class="custom-file-control"></span>
   

   Yeah its here, we already translate the stuff so we should not need CSS selector based on language.

+++ b/themes/bs_bootstrap/css/components/forms.css
@@ -265,13 +265,222 @@ select.form-control-lg:not([size]):not([multiple]) {
+.custom-file-control:lang(en)::after {
+  content: "Choose file...";
+}

We still have this stuff in generated CSS.

Why don't you just override $custom-file-text BS var, and set it to empty map or null what ever works better?

Couple of small things still...

1. +++ b/themes/bs_bootstrap/sass/variables/_bs_bootstrap.scss
   @@ -5,3 +5,9 @@
   +// Removes placeholder text for file upload, we use the translated values from the twig template.
   

   Missing line break in comment because of more then 80 lines.

2. +++ b/themes/bs_bootstrap/sass/variables/_bs_bootstrap.scss
   @@ -5,3 +5,9 @@
   +$custom-file-text: (
   +  placeholder: (null),
   +  button-label: (null)
   +);
   

   This override should actually go to _bootstrap.scss because this is not custom variable form bs_bootstrap theme.

   And don't forget !default property.

1. +++ b/themes/bs_bootstrap/bs_bootstrap.libraries.yml
   @@ -126,10 +126,3 @@ tab:
   -
   -scrollspy:
   -  js:
   -    js/bootstrap/scrollspy.js: {}
   -  dependencies:
   -    - bs_bootstrap/nav
   -    - bs_bootstrap/util
   

   Why is this removed?

2. +++ b/themes/bs_bootstrap/gulp-options.yml
   @@ -30,4 +30,3 @@ bootstrap:
   -    - 'js/dist/scrollspy.js'
   

   Here also, removed?

3. +++ b/themes/bs_bootstrap/sass/components/forms.scss
   @@ -1,11 +1,22 @@
   -.form-actions {
   -  margin-top: 1.5 * $spacer-y;
   -  margin-bottom: 1.5 * $spacer-y;
   

   This also look as a code that is removed but its not related to this issue, why? If this is really not needed and vertical spacing between form elements is fine, then its fine.

Here is a new patch.

1, 2, 3 from comment 22 addressed.

Beside that next things are done

• Fixed theter copy task and reordered js file options a bit.
• Improvements for form-group CSS.
• Improvements for details Drupal control - now its rendered a bit better, for example multi file upload field.
• Fixed height of input fields in details container.

@Denchev can you please do review and test of this - i would really like to close this issue properly sooner then later ;)

1. and 2. we already have an issue for that #2878378: Visual look of multi file field is not good - can you move this stuff there please? Also we should not do 2878378 until #2868082: Add BS responsive table support is done because we need proper responsive table support first for multifields also.

> 3.

bs_bootstrap is not an admin theme i would say, but for sure it should support admin forms as much as possible. Can you create a follow up for this for alpha2, and if we want to support admin forms better then we need to do also drop button support better - that 'Save and keep published' button - can you add one more follow up for this in alpha2 meta issue.
Also no clearfix hacks please any more - they are needed for float layout model which we do not use. It is more likely that that vertical tabs have float somewhere which is messing with layout - we should remove that float if possible and change it with flex model. If this is too much work then its fine we temporary add clearfix for this.

> 4.

Can we move this also to #2878378: Visual look of multi file field is not good or to a separate follow up for alpha2.

Tested this in detail one more time - got some more ideas while reviewing code and created couple of more issues :)

Also improved form layout a little bit more - added vertical margins between form items and similar.

This is committed finally (jay :) and all followups are looking good so closing this one, thx for all the work.