Moodle
  1. Moodle
  2. MDL-29787

Custom Menu in Overlay theme single menu item not styled correctly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.5, 2.1.2, 2.2
    • Fix Version/s: 2.0.6, 2.1.3
    • Component/s: Themes
    • Labels:
      None
    • Testing Instructions:
      Hide

      1. Select Overlay theme in normal way via Theme Selector
      2. Add menu items to Custom Menu in Theme settings' page as per instructions using the example given on that settings page, making sure at least ONE top level menu item with a hyper-link exists but has NO sub-menus added; For Example
      TEST|http://tracker.moodle.org
      3. Test that when this single item is hovered over that there is NO blue image background visible, only a "Dark Gray" a shade lighter than the menu bar.
      4. Test the remaining menu items, which have sub-menus, that they show a horizontal arrow ">" to indicate sub-menus exist and a vertical "v" arrow when selected/hovered over.

      Show
      1. Select Overlay theme in normal way via Theme Selector 2. Add menu items to Custom Menu in Theme settings' page as per instructions using the example given on that settings page, making sure at least ONE top level menu item with a hyper-link exists but has NO sub-menus added; For Example TEST|http://tracker.moodle.org 3. Test that when this single item is hovered over that there is NO blue image background visible, only a "Dark Gray" a shade lighter than the menu bar. 4. Test the remaining menu items, which have sub-menus, that they show a horizontal arrow ">" to indicate sub-menus exist and a vertical "v" arrow when selected/hovered over.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull Master Branch:
    • Rank:
      19300

      Description

      Single menu items show a blue gradient image when hovered over. This fix is to disable that 'sprite' image which is added as a default background by YUI CSS.

        Activity

        Hide
        Mary Evans added a comment - - edited

        Hi Sam,

        I have added you as a watcher for this Tracker Issue which I opened and have just created a fix which I have just submitted.

        I think I have managed to block the hover image without losing the vertical arrows you added earlier this year.
        Apparently there were no horizontal styles added which style a single top-level item.

        This works in IE.

        I know you have spent time with this menu...and perhaps as fed up with it as we all are with the IE white overlay problem.

        Thanks
        Mary

        Show
        Mary Evans added a comment - - edited Hi Sam, I have added you as a watcher for this Tracker Issue which I opened and have just created a fix which I have just submitted. I think I have managed to block the hover image without losing the vertical arrows you added earlier this year. Apparently there were no horizontal styles added which style a single top-level item. This works in IE. I know you have spent time with this menu...and perhaps as fed up with it as we all are with the IE white overlay problem. Thanks Mary
        Hide
        Sam Hemelryk added a comment -

        Hi Mary,

        I've just been looking at this now and have been trying to replicate the problem however I have been unable to replicate it on master.
        I then tested 21 and 20 and could replicate it there however I think the solution you have could do with a little more work sorry.

        First up the three new selectors added for background-color #666 arn't needed or actually applied:

        #custommenu .yui3-skin-sam .yui3-menu-horizontal .yui3-menu-label .yui3-menu-label-active,
        #custommenu .yui3-skin-sam .yui3-menu-horizontal .yui3-menuitem-active .yui3-menuitem-content,	
        +#custommenu .yui3-skin-sam .yui3-menu-horizontal .yui3-menu-label-menuvisible
        

        #custommenu is a div within yui3-skin-sam (not outside) so those new selectors can be removed.

        Next:

        .yui3-skin-sam .yui3-menu-horizontal .yui3-menuitem-active .yui3-menuitem-content {
            background-image: none!important;
            border: 0;
        }
        

        I worked really hard to elliminate the use of !important from the custom menu - its not needed and usually only represents a problem. In this case it is the lack of an id in the selector. The reason that I wrapped the custom menu in a div with an id='custommenu' was so that we always had a something to use in a selector that would ensure precedence.
        If you change your rule to the following it should work just as well:

        #custommenu .yui3-menuitem-active .yui3-menuitem-content {
            background-image: none;
            border: 0;
        }
        

        I was having a bit of a play and I think that change is all you need. I don't think the border rule is required thought .

        I'm sending it back this week sorry so that we can tidy it up. It will need to be tested in all branches on at least Firefox, Chrome, and IE.

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Hi Mary, I've just been looking at this now and have been trying to replicate the problem however I have been unable to replicate it on master. I then tested 21 and 20 and could replicate it there however I think the solution you have could do with a little more work sorry. First up the three new selectors added for background-color #666 arn't needed or actually applied: #custommenu .yui3-skin-sam .yui3-menu-horizontal .yui3-menu-label .yui3-menu-label-active, #custommenu .yui3-skin-sam .yui3-menu-horizontal .yui3-menuitem-active .yui3-menuitem-content, +#custommenu .yui3-skin-sam .yui3-menu-horizontal .yui3-menu-label-menuvisible #custommenu is a div within yui3-skin-sam (not outside) so those new selectors can be removed. Next: .yui3-skin-sam .yui3-menu-horizontal .yui3-menuitem-active .yui3-menuitem-content { background-image: none!important; border: 0; } I worked really hard to elliminate the use of !important from the custom menu - its not needed and usually only represents a problem. In this case it is the lack of an id in the selector. The reason that I wrapped the custom menu in a div with an id='custommenu' was so that we always had a something to use in a selector that would ensure precedence. If you change your rule to the following it should work just as well: #custommenu .yui3-menuitem-active .yui3-menuitem-content { background-image: none; border: 0; } I was having a bit of a play and I think that change is all you need. I don't think the border rule is required thought . I'm sending it back this week sorry so that we can tidy it up. It will need to be tested in all branches on at least Firefox, Chrome, and IE. Cheers Sam
        Hide
        Mary Evans added a comment -

        Hi Sam,

        That's OK...I appreciate you looking at this.
        It's ages since I worked on the Custommanu, and did find while some selectors worked others didn't, yet when you took them away...it went haywire! LOL

        I'll play with this a little more and resubmit next week.

        Cheers

        Show
        Mary Evans added a comment - Hi Sam, That's OK...I appreciate you looking at this. It's ages since I worked on the Custommanu, and did find while some selectors worked others didn't, yet when you took them away...it went haywire! LOL I'll play with this a little more and resubmit next week. Cheers
        Hide
        Mary Evans added a comment -

        @Sam
        I see now what you were explaining to me about the menu.

        I have been working through the custommenu CSS and broke it down into single lines of code to see how the different background, link, hover and active colours work. All fascinating stuff, and so I can appreciate now the need for the ID selector #custommenu to be added to each CSS as this does make a vast difference and does away with the need for "important!" which seems to be used far too much in some Moodle themes.

        I've just added the one line as you said...and it works!

        Thanks
        Mary

        Show
        Mary Evans added a comment - @Sam I see now what you were explaining to me about the menu. I have been working through the custommenu CSS and broke it down into single lines of code to see how the different background, link, hover and active colours work. All fascinating stuff, and so I can appreciate now the need for the ID selector #custommenu to be added to each CSS as this does make a vast difference and does away with the need for "important!" which seems to be used far too much in some Moodle themes. I've just added the one line as you said...and it works! Thanks Mary
        Hide
        Aparup Banerjee added a comment -

        just correcting the repo links

        Show
        Aparup Banerjee added a comment - just correcting the repo links
        Hide
        Aparup Banerjee added a comment -

        this has been integrated test away!

        Show
        Aparup Banerjee added a comment - this has been integrated test away!
        Hide
        Rossiani Wijaya added a comment -

        This is working great.

        Test passed.

        Show
        Rossiani Wijaya added a comment - This is working great. Test passed.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        And this has been sent upstream (already available @ git and cvs repos). Many, many thanks!

        Closing as fixed, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - And this has been sent upstream (already available @ git and cvs repos). Many, many thanks! Closing as fixed, ciao

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: