Admin menu dropdown in Joomla! 3.6.0 RC1 #10987

cyrezdev · 2016-07-01T16:59:38Z

Pull Request for Issue #10972.

Summary of Changes

Fix missing pointer in dropdown menu
Fix sub-menu top placement

Testing Instructions

See issue Admin menu dropdown in Joomla! 3.6.0 RC1 #10972 for more details

This reverts commit 6cd63ab.

brianteeman · 2016-07-01T17:08:31Z

I have tested this item ✅ successfully on daa3d4d

All good

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

brianteeman · 2016-07-01T17:11:41Z

@JoomliC bad memory I guess I thought the other PR was yours - thanks for fixing it though

@C-Lodder can you confirm

cyrezdev · 2016-07-01T17:12:58Z

@JoomliC bad memory I guess I thought the other PR was yours - thanks for fixing it though

This what i thought ;-)
Thanks for test!

StefanSTS · 2016-07-01T17:57:17Z

Tested and works. Applied Commit SHA: daa3d4d with patch tester.

wojsmol · 2016-07-01T19:34:00Z

@StefanSTS please mark as tested https://issues.joomla.org/tracker/joomla-cms/10987 (after login the Test button appears). See https://docs.joomla.org/Testing_Joomla!_patches#Recording_test_results

StefanSTS · 2016-07-01T21:42:55Z

I have tested this item ✅ successfully on daa3d4d

Thanks for the links. Works as expected.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

brianteeman · 2016-07-01T21:53:55Z

Rtc

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

brianteeman · 2016-07-01T21:54:30Z

Setting for 3.6 as it fixes a bug introduced in 3.6

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

infograf768 · 2016-07-02T06:31:11Z

Not sure it should be RTC.
RTL needs more care, before as well as after patch.
Position of sub and arrow.

Before patch

After patch

infograf768 · 2016-07-02T09:41:42Z

Would already be better imho (although the arrow will not display exactly as it should) without

.navbar .nav > .dropdown.open:after {
    right: 10px;
}

for rtl

cyrezdev · 2016-07-02T11:25:35Z

@infograf768 you're right, i should have checked on RTL!
I remove the rtl declaration for pointer.
There are issue in RTL not yet reported, and i thought it was ok in RTL as PR #9375 was already merged (but not sure it was tested on RTL...)

So, the position top and arrow not displayed is fixed now.
But 2 other issues in RTL : placement of pointer & placement of sub-menu.

It seems that template.js is not working to get correctly the offset (as i suspected here #10972 (comment) there is maybe something wrong in the script...) @C-Lodder any idea ? (as you did the PR, your help is welcomed, as i don't see yet how to fix it, and keep your auto-scrolling feature...)

joomla-cms-bot · 2016-07-02T11:26:34Z

This PR has received new commits.

CC: @brianteeman, @StefanSTS

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

joomla-cms-bot · 2016-07-02T11:27:14Z

This PR has received new commits.

CC: @brianteeman, @StefanSTS

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

brianteeman · 2016-07-02T11:28:15Z

Removed rtc

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

Twincarb · 2016-07-02T12:04:59Z

I have tested this item ✅ successfully on 8c324ba

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

cyrezdev · 2016-07-02T12:38:26Z

@Twincarb not to be tested again yet, as still an issue in RTL that should be fixed ;-)

Twincarb · 2016-07-02T14:01:47Z

I have tested this item 🔴 unsuccessfully on 8c324ba

Changed my result having read down to the first successful tests then paged down to the bottom with the extra commits missing infografs input.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

joomla-cms-bot · 2016-07-02T17:48:45Z

This PR has received new commits.

CC: @brianteeman, @StefanSTS, @Twincarb

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

This reverts commit 9676831.

This reverts commit 8c324ba.

joomla-cms-bot · 2016-07-02T17:52:03Z

This PR has received new commits.

CC: @brianteeman, @StefanSTS, @Twincarb

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

cyrezdev · 2016-07-02T17:57:59Z

@infograf768 @Twincarb @brianteeman @StefanSTS i have updated this PR to improve a bit the script, and to fix the issue with submenu placement in RTL reported by @infograf768

If you could test again this PR ? (in LTR and RTL language)

Thanks everybody!

Note: @C-Lodder if you have time to look at code, i have redo a bit your script code in template.js, to set the offset of the submenu with no hardcoded value, and changing a bit the way it is handled. Thanks! ;-)

dgrammatiko · 2016-07-02T18:18:27Z

I have tested this item ✅ successfully on d7442a8

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

infograf768 · 2016-07-03T06:32:53Z

@JoomliC
I also made a PR with simpler changes to template.js.
https://www.dropbox.com/s/lmbobjg8rj5ly5v/admin_menu.diff?dl=0

diff --git a/administrator/templates/isis/js/template.js b/administrator/templates/isis/js/template.js
index 28eb4ff..d465088 100644
--- a/administrator/templates/isis/js/template.js
+++ b/administrator/templates/isis/js/template.js
@@ -75,6 +75,7 @@
            var dropdown = $self.next('.dropdown-menu');
            var offset   = $self.offset();
-           var scroll   = $(window).scrollTop() + 5;
+           var scroll   = $(window).scrollTop() + 8;
            var width    = menuWidth - 13;
+           var rtlwidth = menuWidth + 10;

            // Set the submenu position
@@ -83,5 +84,5 @@
                emptyMenu.css({
                    top : offset.top - scroll,
-                   left: offset.left - width
+                   left: offset.left - rtlwidth
                });
            }

I get exactly the same results as your PR, but... in both cases I have issues here with the User Menu and some glitches with the bottom link in each menu with a sub.
Here is a screen recording :
https://www.dropbox.com/s/fhlweqzwmq1s07g/admin_menu_rtl.mp4?dl=0

Twincarb · 2016-07-03T07:01:38Z

I have tested this item ✅ successfully on d7442a8

Checked in several rtl languages, and also across Chrome, IE, Edge, Firefox. No issues found having cleared the browser cache. Unable to replicate the issues infograf768 found in his video.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

infograf768 · 2016-07-03T07:49:33Z

@JoomliC
Ok, I found the culprit. All depends on the length of the lang string value in mod_menu.ini
If the value is more than a certain number of characters/glyphs, it breaks the way I show in my video.
For Farsi, which was the language I used,
we have
MOD_MENU_COM_USERS_ADD_LEVEL="ایجاد سطح دسترسی جدید"
and
MOD_MENU_COM_USERS_NOTE_CATEGORIES="مجموعه یادداشت کاربر"

If the value is more than 19 latin characters (or its equivalent in glyphs), in the menu OR the submenu it pushes the submenu by a number of pixels depending on the longest glyph.

cyrezdev · 2016-07-03T15:29:35Z

@infograf768 Do you have recorded your video with this PR updated, or with your own changes in template.js ?
If with your own changes, the issue is expected because of submenu width is not a fixed one and change for each submenu (that's the reason i subtract the submenu width dynamically to adjust the offset left. ;-) )

I don't have any issue with Farsi with this PR as it is now. Could you do a test with only this PR applied and no custom changes ?

Note: in the updated PR, to fix RTL, i get dynamically the link width (menu link to open the dropdown) and then get too its padding left (this is currently 10px, so that's the reason for the need of "10", but better to get this padding dynamically than statically ;-) ).
And if you check the left position set in RTL for submenu in my PR, you will see that i get the submenu width dynamically too (this is to fix the issue you have with your custom template.js ;-) ) :
left: offsetLeft - (menuWidth - linkWidth) - submenuWidth

So, maybe an issue in your re-test of this PR after update ? (As i don't have the issue you have recorded here in Farsi : https://www.dropbox.com/s/fhlweqzwmq1s07g/admin_menu_rtl.mp4?dl=0 but with your custom code for template.js, it is "normal" to have this issue ;-) )

Thanks!
Cyril

infograf768 · 2016-07-03T17:03:26Z

My mistake somewhere among all the tests sites...

RTC. Thanks.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

cyrezdev · 2016-07-03T17:48:18Z

You're welcome @infograf768 👍

Thanks everybody for testing!

infograf768 · 2016-07-04T06:16:04Z

@JoomliC
Please look at #11013
I don't have any test sites here with so many components.

RonakParmar · 2016-07-04T06:28:40Z

I have tested this item 🔴 unsuccessfully on d7442a8

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

RonakParmar · 2016-07-04T06:29:02Z

I have installed latest staging branch in my local and applied this PR, still having same issue.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10987.}

infograf768 · 2016-07-04T06:37:49Z

I confirm issue
#11013

@RonakParmar
No relation with this PR. Your screenshot is fine and unrelated to the issue you found.
I posted my findings on #11013

C-Lodder · 2016-07-05T09:25:58Z

What's happening with this. Do I need to look into anything or is it now RTC

infograf768 · 2016-07-05T09:27:09Z

It is RTC.
@wilsonge

I suggest this goes into next RC.

cyrezdev added 6 commits July 1, 2016 18:27

Fix submenu top position

f4f7868

Fix missing pointer for dropdown-menu

5a4d9cc

Run generates

6cd63ab

Revert "Run generates"

08799e0

This reverts commit 6cd63ab.

Run generates

592ae4d

Run generatecss with missing rtf

daa3d4d

joomla-cms-bot added the PR-staging label Jul 1, 2016

cyrezdev mentioned this pull request Jul 1, 2016

Admin menu dropdown in Joomla! 3.6.0 RC1 #10972

Closed

cyrezdev changed the title ~~Patch 32~~ Fix for issue #10972 (3.6.0-rc) Jul 1, 2016

brianteeman changed the title ~~Fix for issue #10972 (3.6.0-rc)~~ Admin menu dropdown in Joomla! 3.6.0 RC1 Jul 1, 2016

brianteeman changed the title ~~Admin menu dropdown in Joomla! 3.6.0 RC1~~ Fix for issue #10972 (3.6.0-rc) Jul 1, 2016

brianteeman changed the title ~~Fix for issue #10972 (3.6.0-rc)~~ Admin menu dropdown in Joomla! 3.6.0 RC1 Jul 1, 2016

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 1, 2016

brianteeman added this to the Joomla 3.6.0 milestone Jul 1, 2016

Fix pointer display for dropdown-menu

9676831

Revert

8c324ba

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 2, 2016

Fix submenu placement in RTL

b34463e

cyrezdev added 3 commits July 2, 2016 19:49

Revert "Fix pointer display for dropdown-menu"

fe0875a

This reverts commit 9676831.

Revert "Revert"

33cd067

This reverts commit 8c324ba.

Fix dropdown pointer in RTL

d7442a8

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 3, 2016

infograf768 mentioned this pull request Jul 4, 2016

Release blocker? : Not able to see submenu items of component #11013

Closed

wilsonge merged commit 63a4466 into joomla:staging Jul 6, 2016

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 6, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Admin menu dropdown in Joomla! 3.6.0 RC1 #10987

Admin menu dropdown in Joomla! 3.6.0 RC1 #10987

cyrezdev commented Jul 1, 2016 •

edited by joomla-cms-bot

brianteeman commented Jul 1, 2016

brianteeman commented Jul 1, 2016

cyrezdev commented Jul 1, 2016

StefanSTS commented Jul 1, 2016

wojsmol commented Jul 1, 2016

StefanSTS commented Jul 1, 2016

brianteeman commented Jul 1, 2016

brianteeman commented Jul 1, 2016

infograf768 commented Jul 2, 2016

infograf768 commented Jul 2, 2016

cyrezdev commented Jul 2, 2016

joomla-cms-bot commented Jul 2, 2016

joomla-cms-bot commented Jul 2, 2016

brianteeman commented Jul 2, 2016

Twincarb commented Jul 2, 2016

cyrezdev commented Jul 2, 2016

Twincarb commented Jul 2, 2016

joomla-cms-bot commented Jul 2, 2016

joomla-cms-bot commented Jul 2, 2016

cyrezdev commented Jul 2, 2016 •

edited

dgrammatiko commented Jul 2, 2016

infograf768 commented Jul 3, 2016

Twincarb commented Jul 3, 2016

infograf768 commented Jul 3, 2016

cyrezdev commented Jul 3, 2016 •

edited

infograf768 commented Jul 3, 2016

cyrezdev commented Jul 3, 2016

infograf768 commented Jul 4, 2016

RonakParmar commented Jul 4, 2016

RonakParmar commented Jul 4, 2016

infograf768 commented Jul 4, 2016 •

edited

C-Lodder commented Jul 5, 2016

infograf768 commented Jul 5, 2016

Admin menu dropdown in Joomla! 3.6.0 RC1 #10987

Admin menu dropdown in Joomla! 3.6.0 RC1 #10987

Conversation

cyrezdev commented Jul 1, 2016 • edited by joomla-cms-bot

Summary of Changes

Testing Instructions

brianteeman commented Jul 1, 2016

brianteeman commented Jul 1, 2016

cyrezdev commented Jul 1, 2016

StefanSTS commented Jul 1, 2016

wojsmol commented Jul 1, 2016

StefanSTS commented Jul 1, 2016

brianteeman commented Jul 1, 2016

brianteeman commented Jul 1, 2016

infograf768 commented Jul 2, 2016

infograf768 commented Jul 2, 2016

cyrezdev commented Jul 2, 2016

joomla-cms-bot commented Jul 2, 2016

joomla-cms-bot commented Jul 2, 2016

brianteeman commented Jul 2, 2016

Twincarb commented Jul 2, 2016

cyrezdev commented Jul 2, 2016

Twincarb commented Jul 2, 2016

joomla-cms-bot commented Jul 2, 2016

joomla-cms-bot commented Jul 2, 2016

cyrezdev commented Jul 2, 2016 • edited

dgrammatiko commented Jul 2, 2016

infograf768 commented Jul 3, 2016

Twincarb commented Jul 3, 2016

infograf768 commented Jul 3, 2016

cyrezdev commented Jul 3, 2016 • edited

infograf768 commented Jul 3, 2016

cyrezdev commented Jul 3, 2016

infograf768 commented Jul 4, 2016

RonakParmar commented Jul 4, 2016

RonakParmar commented Jul 4, 2016

infograf768 commented Jul 4, 2016 • edited

C-Lodder commented Jul 5, 2016

infograf768 commented Jul 5, 2016

cyrezdev commented Jul 1, 2016 •

edited by joomla-cms-bot

cyrezdev commented Jul 2, 2016 •

edited

cyrezdev commented Jul 3, 2016 •

edited

infograf768 commented Jul 4, 2016 •

edited