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?
| Comment | File | Size | Author |
|---|---|---|---|
| #27 | without-patch-26.png | 28.91 KB | shameemkm |
| #27 | with-patch-26.png | 30.44 KB | shameemkm |
| #26 | 1324058_forum_rtl_icon.patch | 1.13 KB | shameemkm |
| #25 | Before Patch | 28.88 KB | shameemkm |
| #25 | After Patch | 42.24 KB | shameemkm |
Comments
Comment #1
larowlanNeeds to be fixed in 8 first.
Comment #2
isay commentedComment #3
develcuy commentedAdding tag "dlatino" for reference of the Drupal Latino community.
Comment #4
larowlanCan we have a screenshot of the before and after?
Thanks
Comment #5
isay commentedI 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.
Comment #6
arbel commentedLooks good +1
Comment #7
larowlanThanks Idan for a second set of qualified eyes on this.
Comment #8
catchThanks! Committed/pushed to 8.x, moving to 7.x for backport - adding Novice tag for the re-roll.
Comment #9
isay commentedpatch for drupal 7
Comment #10
barrapontoI 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.
Comment #11
barrapontoHere is a patch that fixes that for D8 and adds the /* LTR */ marker in forum.css as well.
Comment #12
barrapontoAnd here's a backport.
Comment #14
crazyrohila commentedChanging to 7.x-dev.
Comment #15
crazyrohila commented#12: 1324058-forum-rtl-indent-is-not-overridden-d7.patch queued for re-testing.
Comment #16
isay commentedCapi (barraponto) you are right.
+1
I check this patch and it is work OK.
Comment #17
isay commentedComment #18
xjmCan we get before-and-after screenshots here? Thanks!
Comment #19
isay commentedthe screenshotss is inserted in comment #5
Comment #20
irunflower commentedThis should stay on 8.x until it has been committed.
Comment #21
irunflower commentedVerified 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.
Comment #22
isay commentedYou 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)
Comment #24
shameemkm commentedRerolled patch
Comment #25
shameemkm commentedD8.x
Applied patch in #24 attached screenshot, New issue the icon seems to be aligned to the left.
Comment #26
shameemkm commentedPatch 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.
Comment #27
shameemkm commentedManual Test Result for the patch in #26
Comment #28
star-szrThanks @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:
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).
Comment #29
shameemkm commented@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.
Comment #30
star-szr@shameemkm - Maybe the icon part doesn't need backporting, but the rest does, see #10.
Comment #31
shameemkm commented@Cottser but there seems to be a backport for that in #12. Should we do that again?
Comment #32
star-szr#12: 1324058-forum-rtl-indent-is-not-overridden-d7.patch queued for re-testing.
Comment #33
star-szr@shameemkm - Good point, let's make sure it still applies.
Comment #34
shameemkm commentedChanging from 8.x to 7.x to test patch #12
Comment #35
shameemkm commented#12: 1324058-forum-rtl-indent-is-not-overridden-d7.patch queued for re-testing.
Comment #36
star-szrAnd 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.
Comment #37
dries commentedCommitted to 8.x. Moving to 7.x for #16. Thanks!
Comment #38
David_Rothstein commentedCommitted to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/37b3f3e
Comment #39.0
(not verified) commentedchange "there is no indent for no containers" to "there is no indent"
because indent use for containers too (in level 2 and next)