Regression: Indented styles for indented comments are missing in Bartik.

@munzirtaha : If we apply this patch then, there will be indentation on both sides which makes UI inappropriate as indentation should be from one side.

Applied the patch in #4 and comments appear indented correctly in both LTR and RTL.

@munzirtaha thanks for contribution.

Comments are now properly indented. No bartik and seven's css code is there for indentation, All are managed through classy itself. There is minor css issue for inline links (delete, edit, reply) for RTL. Is there any separate issue for that or We have to do it in this issue. I am attaching png's, Please check spacing issue in that.

Removing old hashed tag.

@munzirtaha Maybe you did this by mistake but please do not set your own patch to Reviewed & tested by the community. We don't have any screenshots of the site in LTR and we don't have any screenshots of how it looked before to compare. Let's get these first before we site it to RTBC.

@lewis, @munzirtaha had create a new issue #2514734: A forgotten padding-right for a ul made inline links look wrong in RTL .
can we create a new issue for inline links ? OR solve that in this issue itself ?

What should be best approach ?

setting back to needs review as we need a screenshots for intended comments.

I cannot see why we are removing Bartik styles which are different to Classy.

Bartik sets 40px on the left in it's styles.

.comment .indented {
  margin-left: 40px; /* LTR */
}

But due to markup changes to comments, the current selector setting the 40px is not correct.

Can we instead fix the selector and get the Bartik styles working again please.

Patch in #4 is getting rejected.

The patch in #4 needs a reroll plus the work added to it mentioned in #17.

@emma.maria Updated the patch as per your suggestion in #17 and styles are getting applied from the Bartik's comments.css file.

Need some screenshots after #20.

this issue seems fixed (please find screenshots in attachment)

We are working on in Barcelona with joelpitter & chipway

Worked on the issue at Barcelona 2015 with jim005, JBO, rbrissaud,. It looks RTBC.

Tested in Barcelona DrupalCon 2015, added screenshots (before / after) patch in summary section and in comment.

Worked on the issue at Barcelona 2015 with jim005, JBO, chipway. It looks RTBC.

It's woking well after patch here in DrupalCon Barcelona

Considering this also changes RTL styles can we get right to left screenshots to verify it is ok?

Working on followup of my mentorees work at Barcelona 2015:
Task: Printing screenshots for LTR and RTL languages.
2 tests related to Amazon didn't pass anymore on patch, so I asked for retest on 2512468-20.patch

While waiting for test results, I applied the patch locally with no error.
It seems to solve when we add response to an existing comment.

Oups. images too big.

As last test passed (https://qa.drupal.org/pifr/test/1139948) the patch looks ready to go RTBC.

Who could RTBC https://www.drupal.org/node/2512468#comment-10384197 @ced_cht @eboux @nichautenauve @gaelgosset @Gromit06 @WebSenso @benoitdevos @Creatile #drupalconeur

patch tested
indentation ok

Last test passed with 0 failure.
(Previous fails seemed to be because of Amazon services being unavailable)

Thanks all for the testing of the patch in #20. However I noticed something with the code. Bartik should ideally match the CSS selector that Classy uses, see below.

From indented.css within Classy.

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

There should be no visual changes from applying this change.

CSS selector updated according to #47 request

We need a manual testing after applying the changes of #47.

Patch in #48 looks good.

Before applying patch.

After applying patch.

I tested the patch on the 8.0.x branch. I noticed that with and without the patch applied that reply comments are not indenting anymore and that the markup has changed at HEAD. Reply comments to comments are now not printing with the indented class wrapped around them to differentiate them with 1st level comments.
 

 
Postponing this issue and raising a major bug issue for the above.

Related bug issue that postpones this one... #2581407: Remove the obsoletely named Classy library attached to Bartik's comment.html.twig as Bartik uses its own styles. .

@emma.maria are you sure ? I tested it again tonight and it still worked with the patch...

OK after some detective work and thinking in the other issue, this issue can be worked on. Bumping up to major because at HEAD in Bartik right now reply comments do not have any differentiating styles.

Setting to Needs Work as the RTL styles are missing the margin-left reset on them...

+++ b/core/themes/bartik/css/components/comments.css
@@ -108,15 +108,11 @@
+.indented {
   margin-left: 40px; /* LTR */
 }
-[dir="rtl"] .comment .indented {
+[dir="rtl"] .indented {
   margin-right: 40px;
-  margin-left: 0;
-}

Please put back the margin-left: 0 on the RTL styles to cancel out the LTR styles.

here it is

+++ b/core/themes/bartik/css/components/comments.css
@@ -108,15 +108,12 @@
-  margin-left: 0;
-}
-.comment .links {
-  padding: 0 0 0.25em 0;
+  maring-left: 0;
 }

Oops there is a typo! Also can we put back the .comment .links { code back in also. That has nothing to do with this issue.

sorry for the typo

Perfect! Thank you @bendev!

The patch corrects the selector used for indented comments in Bartik and follows the selector used by Classy.

Screenshots below:

LTR

RTL

Discussed with the D8 committers and we agreed with making this an rc target.

Committed and pushed to 8.0.x. Thanks!

Just a tiny tag tidying - cos I want to delete the erroneous tag