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?

Support from Acquia helps fund testing for Drupal Acquia logo

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
FileSize
349 bytes
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
FileSize
78.53 KB
77.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
FileSize
329 bytes

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
FileSize
1.06 KB

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

barraponto’s picture

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

FileSize
39.62 KB
36.11 KB
38.59 KB
38.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

shameemkm’s picture

FileSize
42.24 KB
28.88 KB

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

shameemkm’s picture

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

FileSize
30.44 KB
28.91 KB

Manual Test Result for the patch in #26

star-szr’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.

star-szr’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?

star-szr’s picture

star-szr’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

star-szr’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)