Improve design of language detection and selection settings page

I've take screenshots of the detection an selection admin page.

1. Without content translation enabled

2. With content translation enabled

3. With content translation enabled - 'Customize Content language' option checked

After i talked to Gabor and Boyan, i just added a margin at the bottom of the table-group.
For the last element I had to add a "last"-class to remove the margin bottom right before the save button.
This one needs now a usability review.

No javamascripts to review...

No JavamaScripts to review...

1. +++ b/core/modules/language/language.admin.inc
   @@ -176,9 +176,14 @@ function language_negotiation_configure_form_table(&$form, $type) {
   +  $lastitem = end($form['#language_types']);
   ...
   +    if ($type === $lastitem) {
   

   We don't usually do naming like this. We instead usually write words separately. So $last_item.

2. +++ b/core/modules/language/language.admin.inc
   @@ -176,9 +176,14 @@ function language_negotiation_configure_form_table(&$form, $type) {
   +    $last = "";
   ...
   +      $last = " last";
   
   @@ -225,7 +230,7 @@ function theme_language_negotiation_configure_form($variables) {
   +    $output .= '<div class="form-item table-language-group table-' . $type . '-wrapper' . $last . '">' . $title . $description . $table . '</div>';
   

   IMHO instead of having a separate 'last' variable, have a $classes variable and initialize it above with the string that is already here, and append the 'last' class as needed. That keeps all classes together nicely?

Also needs screenshots (again :) about the solution now.

Cross-post.

Changed the code as proposed.

Screenshots redone also.

1. With Content Translation enabled and checkbox checked

2. With Content Translation enabled

3. Without Content Translation

Added Patch from #8 because it was ignored in the post before.

Added the same patch from #8 again because there it was ignored....

Looks almost perfect. Only one thing:

+++ b/core/modules/language/language.admin.inc
@@ -225,7 +230,7 @@ function theme_language_negotiation_configure_form($variables) {
+    $output .= '<div class="' . implode(' ',$classes) . '">' . $title . $description . $table . '</div>';

Coding style issue there :) Add a space after the comma.

No need to update screenshots for this, just the patch :)

Fixed the coding standard issue.

This was @Bojhan approved [TM] in person, and otherwise looks good as well, so let's get it in :)

+++ b/core/modules/language/language.admin.inc
@@ -176,9 +176,14 @@ function language_negotiation_configure_form_table(&$form, $type) {
+    $classes = array("form-item table-language-group", "table-' . $type . '-wrapper");

This isn't quite right. It is resulting in css like <div class="form-item table-language-group table-' . language_content . '-wrapper last">. It should be $classes = array('form-item table-language-group', 'table-' . $type . '-wrapper'); - just replace the all the double quotes with single quotes.

+++ b/core/themes/seven/style.css
@@ -1856,3 +1856,12 @@ details.fieldset-no-legend {
+/* Admin specific theming */

This comment is way too vague... a lot of the css in this file is admin specific theming! Let's embrace the pattern set by the other sections and call this /* Language UI */ or /* Language Admin UI */.

Also whilst reviewing the patch I was wondering if there was a way to do this only with css selectors and no php.

I can do the codechanges with doubleqoutes and single quotes and the Comment.
But i think the PHP is needed because every module has the possibility to add another table. And in order to support older browsers we need a specific identificator for the last item.

+++ b/core/modules/language/language.admin.inc
@@ -176,9 +176,14 @@ function language_negotiation_configure_form_table(&$form, $type) {
+    $classes = array("form-item table-language-group", "table-' . $type . '-wrapper");

Use single quotes for all the strings, then the concatenation will fix itself :)

As for the PHP-less solution, https://drupal.org/node/61509 is not yet updated to Drupal 8 browser support. I'm not sure if we support older browsers anymore that need a class and cannot do with :last children selector? Should check with a frontender :)

Trying to get that feedback via https://twitter.com/gaborhojtsy/status/385387453739446272 :)

IE9+, so :last is totally good. http://caniuse.com/#feat=css-sel3

Only tagging for possible port to the ie8 module.

Ok, then let's skip the PHP part and apply the selector to CSS :)

So looks like this would be as simple as the attached. (Also unassigned boris since he is not working on this looks like).

Sorry, had a lot of work to cover in the last weeks. :-/ Thx for adapting the patch.

So apparently last-child does not work the way I thought it would. It will not apply, because the element is not the last child of its parent, although it is the last child *that has this class*. What am I doing wrong? Frontend help needed :)

We clould try to use :nth-last-child(), but if I understand right the submit button always will be the last element and before that we always will have a .table-language-group div. If that is the case we can simpy apply a negative top margin to the submit button.

Yeah well, that assumes some things about the form :D But it is true the bottom margin did too. Those who want to alter this form can adjust the styling as well in overrides.

Tested manually. Looks good as it can.

If there would be a reason to stick to a css :last solution, this works too:

.page-admin-config-regional-language-detection .table-language-group:nth-last-of-type(2) {
  margin-bottom: 0;
}

:)

Oops!

Before/after in one picture. Putting into summary.

Removed needs screenshot tag - the really nice screenshot is already in the issue summary.

Wow, there are a lot of comments in this issue for the change. :) It looked intimidating, but looks like it's a very simple fix.

So!

Committed and pushed to 8.x. Thanks!

Yay, thanks!

Simplification since #type item has been fixed for #states.

Updated issue summary.

Sorry totally wrong issue, meant to send a patch for #2099577: Follow-up: First installer page has initially needless description for how translations are handled.

Hi, can someone involved in this issue review #2194807: Remove obsolete CSS for language admin form please?