Stable theme does not respect "visually hidden" field label display option

I'm +1 for this, ping me if you'd like a hand with a test. This utility class is added all over the place throughout core and though I'd prefer if we had it configurable, especially in stable. It is what it is and should be working in all cases it's used by site builders. Themers can rewrite the CSS if new techniques arise, but the class name is pretty much stuck for 8.x

Forgot about this, tests were kinda tricky to right but this should do the trick.

In overall this looks good even though there's a markup change inside Stable theme. This is a missing functionality inside Stable theme and should be fixed there, even though there is a small risk that we might break some themes.

+++ b/core/modules/field/src/Tests/FieldDisplayTest.php
@@ -0,0 +1,121 @@
+    $renderer = \Drupal::service('renderer');
+    $content = $renderer->renderPlain($build);
+    $this->setRawContent((string) $content);

No need to do this manually since we could use $this->render()

@lauriii actually $this->render() renders the whole page, and gives some false positives because the .visually-hidden class is added in the page template for the "skip content" link. Also, it's slower because this only renders the thing in question and not the rest of the page.

Thanks for the review.

Tested this manually using Stark. The visualy-hidden class is now applied to the visually-hidden field labels:

--- /dev/null
+++ b/core/modules/field/src/Tests/FieldDisplayTest.php

@@ -0,0 +1,121 @@
+namespace Drupal\field\Tests;

I think the test should live in the Drupal\tests namespace and inside the tests/src/Kernel folder

Thanks @lauriii again that makes sense, here's my attempt at that move and removed a couple of modules that weren't needed for the test and array syntax update to short.

Before
Performed all Steps to reproduce
Screenshot showing expected results - Title displays, visually-hidden class not applied

Patch #10 applied to 8.6.x-dev

After
Screenshot showing expected results - Title no longer display, visually-hidden class applied

1. +++ b/core/modules/field/tests/src/Kernel/FieldDisplayTest.php
   @@ -0,0 +1,118 @@
   +    $this->display = entity_get_display($this->entityType, $this->bundle, 'default');
   

   entity_get_display() is deprecated. We should seek for an alternative approach that is not deprecated.

2. +++ b/core/modules/system/templates/field.html.twig
   @@ -53,7 +53,7 @@
   +    <div{{ title_attributes.addClass(label_display == 'visually_hidden' ? 'visually-hidden' : '') }}>{{ label }}</div>
   
   +++ b/core/themes/stable/templates/field/field.html.twig
   @@ -51,7 +51,7 @@
   +    <div{{ title_attributes.addClass(label_display == 'visually_hidden' ? 'visually-hidden' : '') }}>{{ label }}</div>
   

   We should split this to multiple lines since this line is quite line after these changes. We can change to {{- label -}} which will remove any of the whitespaces from the end result.

addressing #15

For #15.1 I just looked up the API docs, saw there were two examples for what to replace it with, and cribbed the first one.

For #15.2, splitting out {{- label -}} still left the opening DIV as the longest line in the file, at over 100 chars. So I copied the way Classy's field template sets the title classes.

Fixing test failure in #16. It needed the second of the suggested approaches for replacing entity_get_display().

Patch applies for me and works on latest pull of 8.6.x dev branch. Attaching two screenshots of before and after. Followed steps above and "Image" label disappears after patch is applied. Marking as Reviewed!

This fixes another deprecation, since #2849669: Fix \Drupal\Component\Utility\Unicode() because of the Symfony mbstring polyfill went in this week.

The patch from #21 addresses the test failure. It only replaces the deprecated method with the recommended function call.

As this has been manually tested by @if-jd, the latest patch only changes a test, and all the tests are now passing, I'm setting this back to RTBC.

+++ b/core/modules/field/tests/src/Kernel/FieldDisplayTest.php
@@ -0,0 +1,126 @@
+    $this->display = \Drupal::entityManager()

The \Drupal::entityManager() is deprecated as well. We should use \Drupal::entityTypeManager() instead.

Fixes #24

Patch #25 works on 8.6.x-dev.
All tests passed.
Also manually tested.

Restoring status to RTBC after #2973992: Permission issue in Nightwatch step marks all full testruns as unstable

Committed bc1b1fa and pushed to 8.6.x. Thanks! 🎉