Refactor forum module CSS files inline with our CSS standards

@LewisNyman - why didn't you upload a patch? What is forum.module.css_.txt other than the current forum.module.css?

@mgifford It's easy to review it in Dreditor to find problems to fix

Ok, no problem. So this would be to just replace core/modules/forum/css/forum.theme.css if everyone's happy with the CSS.

There is loads to do :P

1. #forum .description {
   

   This should be .forum__description

2. #forum td.created,
   #forum td.posts,
   #forum td.topics,
   #forum td.last-reply,
   #forum td.replies,
   #forum td.pager {
   

   forum__created
   forum__posts
   forum__topics
   forum__last-reply
   forum__replies
   forum__pager

3. #forum .icon{
   

   forum__icon

4. #forum .title {
   

   forum__title

5. #forum div.indent {
   

   hmm don't we already have an indent class that we use in menus?

6. #forum .topic-status-new {
   

   forum__topic-status--new

7. #forum .topic-status-hot {
   

   forum__topic-staus--hot

8. #forum .topic-status-hot-new {
   

   forum__topic-staus--hot-new

9. #forum .topic-status-sticky {
   

   forum__topic-staus--sticky

10. #forum .topic-status-closed {
    

    forum__topic-staus--closed

11. [dir="rtl"] .forum-topic-navigation {
    

    forum-topic-navigation is a good class name imo, but why only rtl styling? Where is the ltr styling?

12. [dir="rtl"] .forum-topic-navigation .topic-previous {
    

    forum-topic-navigation__previous

13. [dir="rtl"] .forum-topic-navigation .topic-next {
    

    forum-topic-navigation__next

14. .action--forum {
    

    I'm not sure what this is but I don't think we have an action class in D8, so we can't add a forum modifier to it

@LewisNyman replaced classes in patch.. Please verify

other, what should we do with #forum div.indent, I guess ID's would be no longer in D8.

This is looking good, but we also need to change the twig files as well as the CSS. Setting back to needs work.

CSS changes::
1. Following the last patch of Vidushi Mehta, I noticed there are other css-files inside Customtheme and Bartik that use the #forum-selector.
2. I saw that the #forum td.created and the #forum td.pager selector was no longer being used, so I deleted it from the CSS.
3. Replaced #forum div.indent with .forum .indented following the comment-module.
4. Removed all css using the .forum-topic-navigation selector because I can't find where this is being used? (That's probably why only the rtl was still inside the CSS?)

Twig changes:
1. I updated the template files in the classy theme.

Other changes:
1. There is still a need for a refactoring inside the forum frontend. (but maybe this should be for a other ticket?)
2. .action--forumis now replaced with .forum__action Because this makes more sense to me, or am I missing something here?

+++ b/core/modules/forum/css/forum.theme.css
@@ -23,48 +23,35 @@
+}
\ No newline at end of file

+++ b/core/themes/bartik/css/components/forum.css
@@ -1,6 +1,6 @@
+}
\ No newline at end of file

We are missing blank lines at the end of these files

+++ b/core/modules/forum/css/forum.theme.css
@@ -23,48 +23,35 @@
-.action--forum {
+.forum__action {
   list-style: none;
-}

+++ b/core/modules/forum/src/Controller/ForumController.php
@@ -260,7 +260,7 @@ protected function buildActionLinks($vid, TermInterface $forum_term = NULL) {
-          '#attributes' => ['class' => ['action--forum']],
+          '#attributes' => ['class' => ['forum__action']],

@@ -286,7 +286,7 @@ protected function buildActionLinks($vid, TermInterface $forum_term = NULL) {
-          '#attributes' => ['class' => ['action--forum']],
+          '#attributes' => ['class' => ['forum__action']],

Instead of using custom classes and CSS, I think we can just reuse the .action-links class

There is still a need for a refactoring inside the forum frontend. (but maybe this should be for a other ticket?)

What is left to do?

@LewisNyman adding blank line at the end of file. Please verify.

what should we do, Can we replace .forum__action with.action-links in css and ForumController.php ?

Changes from patch #17:
We now use the .action-links class.

@lewis: Not sure if this has been noticed before, but the comment.theme.css file defines the following:

.indented {
  margin-left: 25px; /* LTR */
}
[dir="rtl"] .indented {
  margin-left: 0;
  margin-right: 25px;
}

This causes the elements with class .indented to have margins defined from the moment the comment-module is being used. I don't think that is a desired behaviour, because we use the .indented also in the forum-module... (Follow-up ticket for this?)

About the frontend refactoring:
We didn't do any semantic reordering in this ticket, so why do we change

#forum td.created,
#forum td.posts,
#forum td.topics,
#forum td.last-reply,
#forum td.replies,
#forum td.pager {
  white-space: nowrap;
}

into

.forum__created,
.forum__posts,
.forum__topics,
.forum__last-reply,
.forum__replies,
.forum__pager {
  white-space: nowrap;
}

and not into

.forum td {
  white-space: nowrap;
}

Something else I noticed is that we can now have a .forum into .forum
Because in forums.html.twig we have:

{% if forums_defined %}
  <div class="forum">
    {{ forums }}
    {{ topics }}
    {{ topics_pager }}
  </div>
{% endif %}

and in forum-list.html.twig we have

<td {% if forum.is_container == true -%}
        colspan="4" class="container"
      {%- else -%}
        class="forum"
      {%- endif -%}>

And last but not least, the testing is failing because "I think" we deleted the id="forum" and now the bot can't create testnodes.

This causes the elements with class .indented to have margins defined from the moment the comment-module is being used. I don't think that is a desired behaviour, because we use the .indented also in the forum-module... (Follow-up ticket for this?)

Yeah that sounds like we should reduce the scope of that selector, can you create a new issue for it? For this issue, can we change the class to .forum__indented?

.forum td {
  white-space: nowrap;
}

Yeah this is a good improvement, what do you think we should do with .forum__pager? Does it make sense to give it nowrap?

Something else I noticed is that we can now have a .forum into .forum

Can we rename the old .forum class to be .forum-list__forum? I think that is more clear what it's purpose is.

1. Created a follow-up issue for the indented class, for now I suggest to keep .forum .indented (the same as how the comment module does this) This way the follow-up ticket will be more clear.

2. I think .forum__pager or the old #forum td.pager is no longer relevant, and it should have been removed from the first patch of mine, can you confirm?

3. Changed the old .forum to .forum-list__forum as suggested.

Also changed ticket to needs review so it is more clear what changes need to happen in the tests.

Working on it.

Here is the interdiff between 23 and 26

Here are the screenshots to compare before and after

Before:

After:

+++ b/core/modules/forum/src/Tests/ForumTest.php
--- a/core/modules/forum/templates/forums.html.twig
+++ b/core/modules/forum/templates/forums.html.twig

+++ b/core/modules/forum/templates/forums.html.twig
@@ -17,7 +17,7 @@
-  <div>
+  <div class="forum">

I don't think we should be adding this class in the module. Only in Classy

Resetted

+++ b/core/modules/forum/src/Tests/ForumTest.php
--- a/core/modules/forum/templates/forums.html.twig
+++ b/core/modules/forum/templates/forums.html.twig
+++ b/core/modules/forum/templates/forums.html.twig
@@ -17,7 +17,7 @@
-  <div>
+  <div class="forum">

in latest patch based upon #27

Thanks, almost done:

+++ b/core/modules/forum/css/forum.theme.css
@@ -23,48 +18,32 @@
-.action--forum {
-  list-style: none;
-}
+}
\ No newline at end of file

We are missing a blank link here

@lewisnyman Adding blank line :)

Excellent, thank you!

Seems legit...

Committed 1cdef09 and pushed to 8.0.x. Thanks!

Thanks for adding the beta eval to the issue summary.