Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
RTL looks good, but hovering over top level menu items shows no dropdowns... No JS error at all. In Firebug I see the
- style change from left:-999 to left:auto, but Im wondering if this should be reversed, e.g. right:-999 to right:auto?
Comment | File | Size | Author |
---|---|---|---|
#86 | border.png | 2.31 KB | hejazee |
#74 | admin_menu-fix_rtl_dropdown_with_admin_menu_toolbar_also-725840-74.patch | 502 bytes | samaphp |
#57 | admin_menu-fix_rtl-725840-57.patch | 4.29 KB | RoySegall |
#46 | patch_for_hebrew_admin_menu.patch | 1020 bytes | zvischutz |
#44 | admin_menu.zip | 4.06 KB | bloggz |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedI've found out that the JavaScript part messes up the offSetLeft value (position) for the dropdown menu div. In LRT a value may be offSetLeft: 318, the same menu in RTL gets a value of offSetLeft: -90 (clearly, that means off screen).
Have to dig deeper to find the wrong calculation.
Comment #2
nimazuk CreditAttribution: nimazuk commentedsame problem.
In RTL languages FF3.6 does not expand sub-menus.
It works fine in IE7.
Comment #3
Oren_Held CreditAttribution: Oren_Held commentedThe whole off-left method is very bad for RTL. I suggest changing it to off-top instead.
For more details, see a post I've just wrote about this issue http://www.held.org.il/blog/?p=260
Comment #4
sunMy experience with RTL is limited. First of all, I need qualified input.
In LTR, elements that exceed the max-width of BODY cause a horizontal scrollbar. Therefore
right: -999em
does not work.Do browsers flip the entire rendering logic? So aforementioned
right: -999em
will not cause a scrollbar, butleft: -999em
will? (I doubt that + recall that I tested it once)If they are not, then this issue is caused by our RTL styles, but the involved JS does not account for the flipped RTL styles. This means that we need to update the JS to remove the styles instead of resetting them to
auto
.Comment #5
W.M. CreditAttribution: W.M. commentedI experience the same with Administration menu 6.x-3.0-alpha4 on a RTL site. I hope that this will be fixed in future releases. It looks like a nice add on.
Comment #6
W.M. CreditAttribution: W.M. commentedWhere do you think the problem is located ?! The js file or the rtl css file ?!
Comment #7
W.M. CreditAttribution: W.M. commentedI would happily test any suggestions / patches. This is a great module and I think we should work together on making it available to all people / all languages.
I am sure the problem is caused by a minor specification in the current code. It shows perfectly but no dropdown on RTL sites.
Comment #8
W.M. CreditAttribution: W.M. commentedFinally SOLVED!
This solution concerns the latest dev version for Drupal SEVEN (November 14th):
a. Replace every "left" with "right" inside the admin_menu.js file
b. I have made small modification to the "admin_menu-rtl.css" file. Look at the attachment. Replace yours with this one.
Developers: please review and see if there are more professional ways to do this.
Admin menu on RTL ! Finally !
Tested with Firefox (3.6.12) and Drupal SEVEN RC-2
Comment #9
W.M. CreditAttribution: W.M. commentedContinues from my last post:
The same applies to the 6.x dev branch. Modify the js file and make similar changes to the respective rtl.css file as outlined above.
Enjoy it!
Comment #10
shlomoweb1 CreditAttribution: shlomoweb1 commentedI understand the logic, but it going to cause me a problem because i have multi-lingual sites, that need rtl and ltr support for administrators!
Comment #11
alayham CreditAttribution: alayham commentedSame issue in 7.x-3.x-dev (Jan 30) in Chrome and Firefox on windows.
Comment #12
Khai Heller CreditAttribution: Khai Heller commentedPlease use the attachment on the next post (#13) do not use this one. TKS
Comment #13
Khai Heller CreditAttribution: Khai Heller commentedHello,
Revised admin_menu_init(), admin_menu.js and admin_menu-rtl.css.
Tested on Google chrome & Opera. Works for me.
You are welcome to give it a try.
Comment #14
alayham CreditAttribution: alayham commentedtested the update in Chrome 10, IE9, and Firefox 4. Drop down menu works finally, but we have some minor issues with second-level sub-menus jumping to the right sometimes, on all tested browsers.
please commit this update.
thanks.
Comment #15
Faissl CreditAttribution: Faissl commentedworks for me for all levels of submenus!
can't this fix be implanted in the next version?
Comment #16
ronyN CreditAttribution: ronyN commentedsolved it with css:
#admin-menu-wrapper { overflow: visible; }
* for 7.x-3.0-rc1 version
Comment #17
sun#1297862: No dropdown menus in RTL language has been marked as duplicate of this issue. Seems to contain #16 as patch.
Comment #18
tsi CreditAttribution: tsi commentedThe fix by ronyN should get commited, I have added the fix to my theme but this is really not the way to do it.
This is only a one-liner but I'll provide a patch if that would help.
BTW Marked #713982: Css property overflow hidden for div will cause admin menu to not show in such themes as a dup
Comment #19
akhayyat CreditAttribution: akhayyat commentedThs fix:
#admin-menu-wrapper { overflow: visible; }
is not enough for version 7.x-3.0-rc3.
It allows the dropdown menus to show, but sub-menus appear on top of parent menus.
Comment #20
Ehud CreditAttribution: Ehud commentedHere is a fix that worked for me. You can add it to one of your theme's css files:
Comment #21
razooloo CreditAttribution: razooloo commentedwork for me
very cool thanks
i add it to admin_menu.css
Comment #22
navid.kashani CreditAttribution: navid.kashani commentedi rewrite rtl style for admin_menu 7.x-3.x, it works perfectly
Comment #23
yogaf CreditAttribution: yogaf commented#22 works fine. thanks
Comment #24
navid.kashani CreditAttribution: navid.kashani commentedupdate patch -> Cleanup & Fix bug in InternetExplorer7
Tested in:
- Firefox
- Chrome
- IE 7,8,9
just one remains problem (edit shortcuts link) that is related to the Drupal core
Comment #25
aflijja CreditAttribution: aflijja commentedi applied the patch #24 and i added in my theme css-rtl :
#edit-shortcuts {
margin-top: 30px;
}
(edit shortcuts link) is ok now.
Tested in (win xp):
- Firefox 13
- Chrome
- opera
- IE 8
Comment #26
akhayyat CreditAttribution: akhayyat commentedAre these fixes for 7.x-3.0-rc3 going to be committed/released any time soon?
Comment #27
navid.kashani CreditAttribution: navid.kashani commentedwe need update rtl style after #806350: Add a search widget to find links in admin menu
Comment #28
tsi CreditAttribution: tsi commented#24 tested on chrome/FF on ubuntu, works fine for me
Comment #29
caspercash CreditAttribution: caspercash commented#24 works for me too! Great! Now its working fine. We can finally use the admin_menu module. Thanks for this great module! Many thanks for the patch!
Comment #30
sunI'm sorry for leaving this bug unattended for so long.
I've committed a first pass to bring admin_menu-rtl.css in line with the recent .dropdown changes.
Getting the full menu to work in RTL is a bit more tricky though. That is, because the JavaScript hard-codes
left
CSS changes, and my attempts to change the JS to conditionally/dynamically use eitherleft
orright
depending oncontext.rtl
blatantly failed.Without that conditional JS, we'd have to change the JS to set a class instead - as in attached patch. However, that has the problem that the class' styles do not override the :hover styles in the same way the
style
attribute on the dropdown lists does. Consequently, the delayed mouseout behavior is broken.Ideally, we'd make the conditional .css() property name work. But a variable cannot be used for that. Need to check whether there are any JS tricks to work around that.
Comment #31
alayham CreditAttribution: alayham commentedlast patch #30 works on a site with RTL theme, although I am still building the site structure in English.
Comment #32
tsi CreditAttribution: tsi commentedCommented about the previous patch (#24 which worked fine), the last one (#30) fixed the issue as well, tested with the dev branch.
Plus, it seems smarter to
.addClass()
then to hardcode.css()
, so patch is RTBC IMOComment #33
hejazee CreditAttribution: hejazee commented#30 works, but has some minimal issues.
I've fixed these issues and created a new patch file which works well.
Please port this patch instead of #30
Comment #34
hejazee CreditAttribution: hejazee commentedCorrected the position of the search results div
Comment #35
odizle CreditAttribution: odizle commented#34 patch seems to be working for me. Thanks.
Comment #36
alayham CreditAttribution: alayham commented#34 worked well for me in 2 projects. I think this is ready to be committed.
Comment #37
layalk#34 worked perfectly!
Comment #38
abn CreditAttribution: abn commented#34: admin_menu-fix_rtl-725840-34.patch queued for re-testing.
Comment #39
alayham CreditAttribution: alayham commentedPatch works, admin menu dropdown works, no visible side effects so far.
Comment #40
talavishay CreditAttribution: talavishay commented#34 worked perfectly!
Comment #41
hejazee CreditAttribution: hejazee commentedHey, The maintainer of this module won't commit this patch (#34) after 7 months?
Comment #42
tsi CreditAttribution: tsi commentedI'm getting tired of applying this patch, here's a patched version of 7.x-3.x
Comment #43
Pere OrgaIMHO This is critical
Comment #44
bloggz CreditAttribution: bloggz commentedI've just edited "admin_menu.js" and it works well! :D
Comment #45
alayham CreditAttribution: alayham commentedTo get the module patched by drush, add the following to your .make file:
Comment #46
zvischutz CreditAttribution: zvischutz commentedI encountered the same error on rc4 version .
Find out that by changing left to right when opening dropdowns it solves the problem for rtl language and it also works on ltr languages.
I attached here a patch file for those who want to use it.
Comment #48
hejazee CreditAttribution: hejazee commented@zvischutz, This issue has been already solved. But the maintainer of this module hasn't committed the latest working patch.
Please see https://drupal.org/comment/reply/725840#comment-6641512
#34 works perfect.
This issue is for latest dev version of this module.
Comment #49
zvischutz CreditAttribution: zvischutz commented@hejazee , Thanks .
I was not aware that all patches are tested here automatically ...
Just published it , if someone wants to continue to work in rc4 version (last official stable version) but still wants this one corrected.
I am new in patch issues , Sorry.
Comment #50
z.stolar CreditAttribution: z.stolar commentedPopping this issue up in the admin|_menu issue queue.
Patch in #34 is ready to be committed and it's function was tested by enough people here.
In the RTL countries we say: "Yalla imshi"
Comment #51
alayham CreditAttribution: alayham commentedtested the patch in #34 again with the recent updates to the dev branch.
Works perfectly. Must be committed asap.
Comment #52
Mehrdad201 CreditAttribution: Mehrdad201 commentedPatch #22 works well. Thank a lot Navid.
Comment #53
heidarzadeh CreditAttribution: heidarzadeh commentedIs this patch committed?
Don't you want to commit it !!!!????
Comment #54
peyman_6000 CreditAttribution: peyman_6000 commentednot working
patch
Comment #55
hejazee CreditAttribution: hejazee commented@peyman_6000 You should use #725840-34: No dropdown menus in RTL language. It works well.
There is no problem with it.
I'm wondering why the maintainer of the module does not apply it for next release !
Comment #56
sinasalek CreditAttribution: sinasalek commentedJust tried patch on comment #34 , applied cleanly and seems to be working fine
Comment #57
RoySegall CreditAttribution: RoySegall commentedRe rolling.
Comment #58
sinasalek CreditAttribution: sinasalek commented#57 applied cleanly against latest dev snapshot.
Making this RTBC
Comment #59
RoySegall CreditAttribution: RoySegall commentedCan we role this one in before we need to re role it again?
Comment #61
samaphpHere is very simple solution .. just edit this file: admin_menu-rtl.css
Line #45
Or in a simple way, put this line in any -rtl.css file:
#admin-menu .dropdown li ul{right: auto;}
Enjoy RTL.
Comment #62
samaphpKindly Review this patch .. it's very simple .. there is no need for manylines to resolve this issue .. only oneline need to be changed to resolve this issue.
Comment #63
drplpatch #62 works fine for me, Thanks
Comment #64
W.M. CreditAttribution: W.M. commentedpatch at #62 fails.
It shows first level children but bot second level ones. The latter appear above current column and they disappear upon hovering over them
Comment #65
3ssom CreditAttribution: 3ssom commentedpatch #62
works like charm even in second levels ,,
Thank you
Comment #66
W.M. CreditAttribution: W.M. commentedIn fact patch listed under #62 solves the issue completely. However, Admin Menu Toolbar still shows issues on RTL sites.
Comment #67
Dave ReidCan someone confirm if this is still an issue with the latest 7.x-3.0-rc5 release?
Comment #68
W.M. CreditAttribution: W.M. commented@Dave Reid
I have applied the patch at #62 to 7.x-3.0-rc5 release. It solved the problem but not under Admin Menu Toolbar.
Comment #69
doxigo CreditAttribution: doxigo commentedand four years later, still I need to apply a patch every time to get this fixed, can anybody please commit a patch to a version?
Comment #70
alayham CreditAttribution: alayham commentedSomebody should close this bug as "won't fix":)
Primarily because of this bug (and for other reasons as well), I dropped this module from my make file. I use navbar
If you are still struggling with admin_menu RTL issues, give navbar a try. It is mobile friendly and works well on RTL and LTR pages.
Comment #71
Dave Reid@alayham: That is unhelpful and unproductive, so stop. Navbar is quite well known and doesn't help to resolve this issue for anyone who can't just up and switch modules (let alone entirely different navigation concepts).
As per #68 this needs a fix for the "Admin Menu Toolbar" styling as well before this can be committed.
Comment #72
alayham CreditAttribution: alayham commentedsource: #1166886: [Policy, no patch] Improve RTL support
Comment #73
alayham CreditAttribution: alayham commentedI tested this patch on the dev snapshot. The patch works and solves the problem.
Patch for "Admin Menu Toolbar" will come later if somebody needs it.
Please be helpful and commit this patch asap. (Remember #72 above)
Comment #74
samaphpFor those who use Admin Menu Toolbar .. here is a new patch to fix the both issues.
So, this patch is should fix the dropdown menu in RTL for everyone who use the (Admin menu toolbar) or not.
@W.M. please be gentle before you write your review and explain why it's not work with you so we can help.
Anyway this patch include the previous patch #62 .. so please be kind and test it and merge this fix please :(
Comment #75
alayham CreditAttribution: alayham commentedI tested this patch on the dev snapshot. The patch works and solves the problem.
I also tested the patch with admin_menu_toolbar, and It works well,
Please be helpful and commit this patch asap. (Remember #72 above)
Comment #76
samaphp@alayham Thanks for review .. your time is really apreciated <3
Waiting now to merge this patch ..
Comment #77
Benia CreditAttribution: Benia commentedIndeed, I hope the patch for the Direction:RTL induced problem would go into the module as soon as possible...
Comment #78
pegahmajma CreditAttribution: pegahmajma commentedThank you @Samaphp it's work fine for me. #74
Comment #79
3ssom CreditAttribution: 3ssom commentedTested patch #74 .. works good
hope its goes into the module soon ..
Thanx @Samaphp
Comment #80
drplHello,
the #74 working fine,
Regards,
Comment #81
hejazee CreditAttribution: hejazee commented#74 looks good, but has a minimal issue (extra borders are visible in admin menu toolbar)
I've fixed this problem and this is the final patch.
Comment #82
hejazee CreditAttribution: hejazee commentedFixed a small typo (misspelled)
Comment #83
hejazee CreditAttribution: hejazee commentedHiding files. set status back.
Comment #84
Dave ReidSince your patch contained changes, please don't RTBC it yourself. Would be good to get confirmation from others that it works and fixes the additional borders.
Comment #85
samaphp@hejazee I think no need for so many changes ..
#74 is just only (502 bytes)
Another thing you just copy what I did then you added an enhancement not fix, so no need enhancement for this in my opinion.
Thanks for your time but I still see the #74 is the best.
Kindly @dave commit these change since we still do this change by hand in every Drupal install :(
Comment #86
hejazee CreditAttribution: hejazee commentedI've attached the images.
This is the result of #74
As you can see, extra borders are visible.
And this is the result of #82
You can see that #82 doesn't have that problem.
@samaphp, here is a community and all of us work and try to enhance the software.
It doesn't matter who does which part of the job.
after all, all of the participants are listed in the commit log in git.
So I believe we should do the best job.
When you post a patch, it may fix the problem, but cause a new problem. (in this case, you fixed the drop down, but extra borders appeared)
But #82 is a better patch since it doesn't expose extra borders. (doesn't produce additional problem)
So you can decide which one you choose.
Thanks all.
Comment #87
alayham CreditAttribution: alayham commented@Hejazee: Your patch solves another problem, not the RTL problem of this case. Please correct me if I am wrong.
I tested #74, and it is the perfect solution for this issue. Other bugs can be reported and solved separately, although they certainly need this patch to be pushed. Therefore, I am changing the status to RTBC for #74 as many of us tested it and it works and solves the problem.
There might be lots of other issues in the RTL admin_menu, In fact, this module was not properly tested because the maintainers ignored fully working patches over in the past years (I wonder why!). and unless the maintainer follows the policy for RTL CSS in D7 (see #72), this bug will take more of our time, and more patches will be submitted to address all other problems in this module's RTL CSS code.
Comment #88
hejazee CreditAttribution: hejazee commentedIt doesn't matter how we solve the problem
I can create a separate issue for "extra borders"
but "extra borders" are directly related to this issue. (it's exposed after applying #74. not before that)
Then we have to wait for this issue to be committed. and then open another issue with another patch and wait for it to be committed.
this may take so much time (as you know a couple of months maybe !!)
------------
but my logic is that if you write a patch and that patch exposes a new problem. and if you can write the patch some other way that it does not expose the problem, it would be much better.
Anyway which of the following do you prefer?:
1 - wait for this issue to be committed. then wait for another issue to be committed. (or you have to apply two separate patches)
2 - apply #82 which works seamlessly and you are good to go.
When you apply #82, you see that the drop-down is fully working with no problems.
---------------
I will apply #82 on all of my sites, because it fixes the problem the best and quickest way. and does not expose a new bug.
but if you're so strict and you want to post two separate issues only for a single line of code, nobody will prevent you.
The maintainer of the module can decide witch patch to apply.
regards.
Comment #89
alayham CreditAttribution: alayham commented@hejazee
I said what I prefer in #70, this issue should be closed as "won't fix" and the maintainer should say clearly admin_menu does not have RTL support, so somebody can create an admin_menu_rtl module.
Besides, my dear, No, the issue is not caused by #74, the issue was invisible until the menu worked after applying #74. I still believe this is a separate bug in the code, and there might be lots of other bugs that won't be found until #74 is pushed.
The real problem here is that the maintainer has been ignoring all the patches submitted and tested during the past 4+ years. The first tested patch was submitted in Dec 15, 2010. You submitted your first patch to this bug in Oct 2012. since then, we have been struggling with the maintainer to commit the patch. Every time the maintainer changed the code, you or somebody else produced an updated patch. This process is not compatible with the RTL CSS policy that I quoted in #72. This is pretty obvious to me, but not to admin_menu maintainer.
Comment #90
drplHello,
I tested #74 in every site I have, and it's working fine.
so please add it in module so save more step during install sites
Regards
Comment #91
alayham CreditAttribution: alayham commentedThis tool can be helpful in converting the entire css file into rtl.
http://cssjanus.commoner.com/
Credit to @LewisNyman for adding a comment about it in #1166886: [Policy, no patch] Improve RTL support
Comment #92
Ardethian CreditAttribution: Ardethian commented#74 works like a charm. Please update the module to include it.
Comment #93
3ssom CreditAttribution: 3ssom commentedevery time I install new drupal I have to apply path #74 .. please add it to the module so we save time
Thank you
Comment #94
yuseferi CreditAttribution: yuseferi commentedpatch #74 resolve my problem, apply it in version
Comment #95
samaphpThis is need to be applied need to be commit in a new release ..
It's so important :(
I've tried to contact Dave Reid by contact form here in drupal.org but there is no response :(
If you guys know anyone please contact with him so we can do our work faster, instead of apply this patch in everytime while we do new Drupal install.
Comment #98
Dave ReidI've committed #74 to 7.x-3.x and 8.x-3.x and am reviewing the rest of the issue queue for things that can be fixed before creating a new release.
Comment #99
Dave ReidNeeds a patch for 6.x-3.x I believe.
Comment #100
hejazee CreditAttribution: hejazee commentedThanks @Dave Reid, please also see this related issue: #2431015: extra borders are visible in admin menu toolbar Assigned to: hejazee
Comment #101
W.M. CreditAttribution: W.M. commented@Dave Reid [7.x] The patch works well, please consider releasing a new full release of Admin Menu, since every time I install it using Drush I find my way again to this post to read that I should had downloaded the development version, little bit annoying :-) Thank you.
Comment #102
3ssom CreditAttribution: 3ssom commentedHello W.M,
I couldn't agree more .. everytime I use Drush I'd have to apply the patch all over again or use the dev. version.
which is really annoying.
Thanx
Comment #103
amagdy CreditAttribution: amagdy commentedFor the love of god, can some one add any of the working patch to the next 7.x release ?
I have been patching it myself every time i install the module for years now
Comment #104
W.M. CreditAttribution: W.M. commentedTrue.. Meanwhile, You can just make a github branch and use it on every new site..
Comment #105
drplHello, please create new release it's annoying to applied this patch every time
Comment #106
samaphpDear Dave Reid, It's been 2 years now since this issue has been fixed.
Please, we need to have this in an official release to save our time.
Waiting for your help.
Comment #107
3ssom CreditAttribution: 3ssom commentedHello Dave,
its about time to have this in a release!
Please make that happens ,, every time I have to apply this when I a have a new installation :(
Thank you
Comment #109
MahmoodZidan CreditAttribution: MahmoodZidan commented#74 works great. Thanks @samaphp :)