Convert theme_details() to Twig

Split from form.inc twig conversion.

Fixed the whitespace errors and did some clean-up.

Shouldn't there be a div.description in here?

+    <div class="details-wrapper">
+      {%- if description -%}
+        <div class="details-description">{{ description }}</div>
+      {%- endif -%}
+      {%- if children -%}
+        {{ children }}
+      {%- endif -%}
+      {%- if value -%}
+        {{ value }}
+      {%- endif -%}
+    </div>
+  </details>

div.details-description does not receive the same text formatting as div.description, which makes form help text look inconsistent, and requires extra work for themes to override.

If there is a div.details-wrapper I'd expect to find a div.details inside that, or else the wrapper should be div.details-description-wrapper.

I think ideally this should be changed to:

<div class="details description">

There are only two lines in core css targeting .details-description (in user.module.css and bartik style.css) - these would need to change to .details.description. Any objections?

@John Pitcairn those are great suggestions though not particularly for this issue. Have a look here and help @mortendk #1980004: [meta] Creating Dream Markup if there is not for theme_details() an issue, you're welcome to create it and add it to the meta with exactly what you wrote in the comment above.

This issue is related to getting the conversion of a theme_* function to a twig file. We won't introduce markup or style changes as we are comparing that we haven't made a mistake in the conversion process.

If you want to help with this issue still, we need someone to compare the markup before and after the patch is applied to make sure we didn't break anything existing. That would help get this theme_ function die and get the template as twig in core so it would be easier for you to manipulate the markup to your liking.

Thought somebody might say that ;-)

Will do. Sorry I can't test this immediately, we are building a site on alpha 7.

Issue is here: #2177813: details.html.twig should include .description class

+++ b/core/modules/system/templates/details.html.twig
@@ -0,0 +1,35 @@
+{% spaceless %}
...
+      {%- endif -%}
+      {%- if children -%}
+        {{ children }}
+      {%- endif -%}
+      {%- if value -%}
+        {{ value }}
+      {%- endif -%}

Due to either spaceless of the minus whitespace removers there is missing space between input's and their labels. My guess is spaceless. I'd rather not use spaceless, but that means fixing the whitespace in the tests.

I'm going to upload a version without the spaceless tag for see exactly which tests fails

After that i'm going to work on fix them.

Starting up the testbot (needs review). To see if there are any tests we need to fix. Hopefully we can do xpath equivalent tests to help ignore whitespace differences. Thanks @gnuget

@gnuget the tests are in-line with the ones you found. It says "ForumUninstallTest" too but I doubt that fail is real(bot anomaly)...

Buena suerte!

Ok, here a patch which replace the failing tests with their xpath equivalents

The rendered code before the patch is the same as the rendered code after the patch. Only the order of some tag attributes and ids or classes has changed, but this does not break the applied css. The page looks exactly the same in the browser.

I haven’t done profiling but i’m not sure if it is necessary for this issue since the theme details won’t be changed often, so performance is not critical.

I profiled the patch in #14. I wasn't sure about the results so I did this multiple times and the results were always similar.

=== 8.x..8.x compared (52e3dfa285188..52e3dffbc04ba):

ct  : 62,040|62,040|0|0.0%
wt  : 377,813|378,779|966|0.3%
cpu : 360,984|361,903|919|0.3%
mu  : 15,181,856|15,197,528|15,672|0.1%
pmu : 15,312,040|15,326,864|14,824|0.1%


=== 8.x..details-twig compared (52e3dfa285188..52e3e019b34d3):

ct  : 62,040|62,242|202|0.3%
wt  : 377,813|376,456|-1,357|-0.4%
cpu : 360,984|359,907|-1,077|-0.3%
mu  : 15,181,856|15,229,112|47,256|0.3%
pmu : 15,312,040|15,358,808|46,768|0.3%

I've also looked at the markup and it looks ok (same as #15).

Now the only thing missing is a proper patch review.

I reviewed the xpath and the patch and did some minor cleanup but I believe this captures all that is expected from the markup without the whitespace differences. Great work.

Oh I did a manual review on this already and reviewed the new xpath changes, so this is good to go. Testbot can disagree if it wants but I think this is RTBC.

Also thanks for profiling @idflood and the test changes @gnuget!

Committed and pushed to 8.x. Thanks!