The problem occur when i enable forum (core forum) and open main forum page (for example www.site-name/forum). The page is in RTL language (hebrew) but there is no indent. in order to fix it next lines of code should be added to forum-rtl.css
#forum div.indent {
margin-right: 20px; /* RTL */
}
Can I fix this problem by myself?
How To do it?

Files: 
CommentFileSizeAuthor
#27 without-patch-26.png28.91 KBshameemkm
#27 with-patch-26.png30.44 KBshameemkm
#26 1324058_forum_rtl_icon.patch1.13 KBshameemkm
PASSED: [[SimpleTest]]: [MySQL] 50,491 pass(es).
[ View ]
#25 Before Patch28.88 KBshameemkm
#25 After Patch42.24 KBshameemkm
#24 1324058-forum-rtl-needs-to-cancel-opposite-margins-reroll.patch646 bytesshameemkm
PASSED: [[SimpleTest]]: [MySQL] 50,391 pass(es).
[ View ]
#21 d7beforechrome.png38.8 KBirunflower
#21 d7afterchrome.png38.59 KBirunflower
#21 d8before.png36.11 KBirunflower
#21 d8after.png39.62 KBirunflower
#12 1324058-forum-rtl-indent-is-not-overridden-d7.patch1.1 KBbarraponto
PASSED: [[SimpleTest]]: [MySQL] 39,749 pass(es).
[ View ]
#11 1324058-forum-rtl-needs-to-cancel-opposite-margins.patch1.06 KBbarraponto
PASSED: [[SimpleTest]]: [MySQL] 35,686 pass(es).
[ View ]
#9 forum-rtl-1324058-9.patch329 bytesisay
PASSED: [[SimpleTest]]: [MySQL] 38,506 pass(es).
[ View ]
#5 with patch.jpg77.15 KBisay
#5 without patch.jpg78.53 KBisay
#2 forum-rtl-1324058-3.patch349 bytesisay
PASSED: [[SimpleTest]]: [MySQL] 34,250 pass(es).
[ View ]

Comments

larowlan’s picture

Version:7.8» 8.x-dev

Needs to be fixed in 8 first.

isay’s picture

Assigned:Unassigned» isay
Status:Active» Needs review
StatusFileSize
new349 bytes
PASSED: [[SimpleTest]]: [MySQL] 34,250 pass(es).
[ View ]
develCuy’s picture

Issue tags:+dlatino

Adding tag "dlatino" for reference of the Drupal Latino community.

larowlan’s picture

Status:Needs review» Needs work
Issue tags:+Needs screenshots

Can we have a screenshot of the before and after?
Thanks

isay’s picture

Status:Needs work» Needs review
StatusFileSize
new78.53 KB
new77.15 KB

I insert the screenshots of main forum page in drupal 7.
You can't see the problem in drupal 8 because there is difference between main forum page in drupal 7 and between main forum page in drupal 8. The difference that in drupal 7 in the main forum page shown all forum topics with it hierarchy. For example if in forum exist the next hierarchy.
Container
--SubContainer1
----SubContainer2
------SubContainer3
---------forum topic1
In drupal 7 in the main forum page you will see table with all hierarchy but in drupal 8 you will see table only with the high level containers and forum topics. For our specific example you will see only Container.
But If in drupal 8 will be option to see all hierarchy like in drupal 7 the same problem will be occurred.

arbel’s picture

Looks good +1

larowlan’s picture

Status:Needs review» Reviewed & tested by the community

Thanks Idan for a second set of qualified eyes on this.

catch’s picture

Title:forum-rtl.css not overide #forum div.indent from margin-left to margin-right» forum-rtl.css does not overide #forum div.indent from margin-left to margin-right
Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)
Issue tags:+Novice, +needs backport to D7

Thanks! Committed/pushed to 8.x, moving to 7.x for backport - adding Novice tag for the re-roll.

isay’s picture

Status:Patch (to be ported)» Needs review
StatusFileSize
new329 bytes
PASSED: [[SimpleTest]]: [MySQL] 38,506 pass(es).
[ View ]

patch for drupal 7

barraponto’s picture

Status:Needs review» Needs work

I think it should cancel the margin-left AND add the margin-right, shouldn't it?
The same applies to the d7 patch.
Of course you're only going to hit this new bug if your topic is huge.

barraponto’s picture

Version:7.x-dev» 8.x-dev
Status:Needs work» Needs review
StatusFileSize
new1.06 KB
PASSED: [[SimpleTest]]: [MySQL] 35,686 pass(es).
[ View ]

Here is a patch that fixes that for D8 and adds the /* LTR */ marker in forum.css as well.

barraponto’s picture

StatusFileSize
new1.1 KB
PASSED: [[SimpleTest]]: [MySQL] 39,749 pass(es).
[ View ]

And here's a backport.

Status:Needs review» Needs work

The last submitted patch, 1324058-forum-rtl-indent-is-not-overridden-d7.patch, failed testing.

crazyrohila’s picture

Version:8.x-dev» 7.x-dev

Changing to 7.x-dev.

crazyrohila’s picture

Status:Needs work» Needs review
isay’s picture

Capi (barraponto) you are right.
+1
I check this patch and it is work OK.

isay’s picture

Status:Needs review» Reviewed & tested by the community
xjm’s picture

Status:Reviewed & tested by the community» Needs review

Can we get before-and-after screenshots here? Thanks!

isay’s picture

the screenshotss is inserted in comment #5

irunflower’s picture

Version:7.x-dev» 8.x-dev

This should stay on 8.x until it has been committed.

irunflower’s picture

StatusFileSize
new39.62 KB
new36.11 KB
new38.59 KB
new38.8 KB

Verified results.

D7.14
Applied patch in #12, worked. Attached screenshots of D7 (in Chrome) before and after. Checked Firefox and Safari, worked as expected.

D8.x
Applied patch in #11. Like #5, you can't see the difference, but patch applied cleanly and I couldn't find any other errors. Screenshots attached.

isay’s picture

You cannot see the difference in drupal 8 because the patch that handle the problem committed/pushed to drupal8 (look comment # 8).
barraponto suggest other fix (read comment 10).
the fix of (barraponto) can be see only when deep hierarchy exist in forums.
for example
container
--subcontainer1
----subcontainer2
------subcontainer3
--------subcontainer4
----------subcontainer5
------------subcontainer6
--------------subcontainer7
----------------subcontainer8
------------------subcontainer9
.... until the name of sub-container is close to right margin (in fact the patch is for RTL language so if example above was in hebrew the name of sub-container should be close to left margin)

shameemkm’s picture

StatusFileSize
new646 bytes
PASSED: [[SimpleTest]]: [MySQL] 50,391 pass(es).
[ View ]

Rerolled patch

shameemkm’s picture

StatusFileSize
new42.24 KB
new28.88 KB

D8.x
Applied patch in #24 attached screenshot, New issue the icon seems to be aligned to the left.

shameemkm’s picture

StatusFileSize
new1.13 KB
PASSED: [[SimpleTest]]: [MySQL] 50,491 pass(es).
[ View ]

Patch to correct the issue mentioned in comment #25 It also includes the rerolled patch from #24 which takes care of the left margin when the language is Right to Left.

I think the icon should also be flipped in case of Right to Left.

shameemkm’s picture

StatusFileSize
new30.44 KB
new28.91 KB

Manual Test Result for the patch in #26

Cottser’s picture

Assigned:isay» Unassigned
Status:Needs review» Reviewed & tested by the community
Issue tags:-Needs screenshots

Thanks @shameemkm! I'm not sure we need to go as far as flipping the icon, at least not in this issue. Feel free to open a new issue if you think the icon should be flipped in the RTL CSS, but that would probably be more of a feature request.

For future reference, you can also embed the screenshots directly into your comments, and Dreditor makes this easy.

Changes and screenshots look good to me, here are the screenshots from #27:

Before:
Before

After:
After

When it comes time to backport this to D7, be sure to add the change from #2 as well (commited to 8.x in #8, not committed to 7.x yet).

shameemkm’s picture

@Cottser I dont think this needs a backport as the icon is not there for container in D7. I think it was introduced in D8.

Cottser’s picture

@shameemkm - Maybe the icon part doesn't need backporting, but the rest does, see #10.

shameemkm’s picture

@Cottser but there seems to be a backport for that in #12. Should we do that again?

Cottser’s picture

Cottser’s picture

@shameemkm - Good point, let's make sure it still applies.

shameemkm’s picture

Version:8.x-dev» 7.x-dev

Changing from 8.x to 7.x to test patch #12

shameemkm’s picture

Cottser’s picture

Version:7.x-dev» 8.x-dev

And this is why you don't re-queue patches at 6am :) Thanks.

Looks like testbot picked up the *-d7.patch, maybe? Either way, test came back green.

8.x patch in #26 and 7.x patch in #12 are both RTBC.

Dries’s picture

Version:8.x-dev» 7.x-dev

Committed to 8.x. Moving to 7.x for #16. Thanks!

David_Rothstein’s picture

Status:Reviewed & tested by the community» Fixed

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary:View changes

change "there is no indent for no containers" to "there is no indent"
because indent use for containers too (in level 2 and next)