Copy forum templates to Classy

moved the classes out of core
Discussion of the classes - should be done as a follow up, so bikesheedding wont stop classy ;)

cause i was stupid ;)

this patch have issues with test its using xpath so killing the ID's & classes gives us issues

Reviewing this now.

Module templates

The following look fine to me, they have the classes and IDs stripped out.

core/modules/forum/templates/forum-list.html.twig
core/modules/forum/templates/forum-submitted.html.twig
core/modules/forums.html.twig

Here is what Stark looks like before and after the patch.
 

 

 

 

My issues with this patch are....

1. +++ b/core/modules/forum/templates/forum-icon.html.twig
   @@ -18,15 +18,9 @@
   -    <a id="new"></a>
   +    <a></a>
   

   Should we leave a completely empty "a" link wrapper here? Doesn't the ID provide a function as an anchor link for new? I would put this back in the module file.

 

2. +++ b/core/modules/forum/templates/forum-icon.html.twig
   @@ -18,15 +18,9 @@
   -  <span class="visually-hidden">{{ icon_title }}</span>
   +  <span>{{ icon_title }}</span>
   

   We need to keep visually-hidden classes they provide a function for accessibility.

 

3. +++ b/core/modules/forum/templates/forum-list.html.twig
   @@ -43,37 +43,35 @@
   -        {% for i in 1..forum.depth if forum.depth > 0 %}<div class="indent">{% endfor %}
   -          <div class="icon forum-status-{{ forum.icon_class }}" title="{{ forum.icon_title }}">
   -            <span class="visually-hidden">{{ forum.icon_title }}</span>
   +        {% for i in 1..forum.depth if forum.depth > 0 %}<div>{% endfor %}
   +          <div title="{{ forum.icon_title }}">
   +            <span>{{ forum.icon_title }}</span>
   

   again visually-hidden needs to be put back in the module file.

 
Otherwise the patch does not break any other functioning of the forum module.

Also the original files from the module have been added to the Classy theme and contain no changes.

Tagging novice for the improvement work said in #14 on the patch in #4.
The work in #14 are small tasks of putting things back into template files that were removed by the patch.

1. hmm do we consider this functionality ? - i can see that it could be ?

2. the visually-hidden is still a a class - its not prefixed as beein essential or with a specific "drupal-stuff" ala js- etc.
Ib core Were assuming absolutely nothing on the frontend. we Dont wanna break the chance for a theme to do completely new things in the future. So yes visually hidden or whatever else unless its absoluterly nessersary for functionality should be removed - its up to a themer to add that it in.

@david true - lets keep them here then take that on as a general issue later on, so we can attacch that to the behaviour of classy

added back the new id & visually-hidden classes

Everything else seems to be okay, but:
1. In classy/templates/forum-submitted.html.twig we say:

{% if time %}
  <span class="submitted">{% trans %}By {{ author|passthrough }} {{ time }} ago{% endtrans %}</span>
{% else %}}</span> 

But I can't see the submitted class in DOM. With Classy installed and enabled:

With Stark installed and enabled:

2. In every file-comment we have line like

 * @see template_preprocess_forum_list()

Do we need these in Classy?

I can't see the class without the patch either, so it seems that either I'm doing something wrong or then we have a place for follow up issue. From my point of view this is RTBC.

this should be followup issue, also i dont really understand the issue ;) but please create it

The patch still applies cleanly, no need to reroll.

Code looks good. I went ahead and did some manual testing:

1. Installed forum module
2. Created a forum node in "General discussion"
3. Verified that classes are still there, no visual regressions.

@DickJohnson that template file is used only on forum listings, not on the final forum node page. So this means it shows up under /forum, /forum/1 and so on. The class appears there fine, so no problems there.

This good to go imho.

Good catch @davidhernandez , here is the updated patch (based on #19).

Oops... forgot to add the new files...

rerolled the patch added back in the attributes that was missed before, good catch :)

huh is testbot drunk ? heres some coffe go test

@davidhernandez thanks for useful link & info, done -=]

I've read through the patch, it looks (even more) beautiful. RTBC'ing

@manuel im using sourcetree it makes it all work for me

Patch works and white space cleaned up

css
took a look at the css as well, the css file should be renamed from forum.module to forum.theme.css as theres no essential css for the forum to work.
im adding this as a followup issue, to get the MAT css fixed / at least have note on it.
#2421099: forum module css file follow MAT naming

Theme changes are not blocked by beta. Committed e6e3034 and pushed to 8.0.x. Thanks!