Greetings.

I'm using your module almost in every project. Recently I found an inconvenience in IE. Since there is no HOVER event for LI item in IE it is impossible to work with hovered LI items. When making a complicated theme there is always a necessity to theme hovered LIs as well as As (for instance, to implement menu like tabs). To solve this problem we propose to add the following lines to set "over" class for hovered LI items:

      $("ul.nice-menu li").hover(function(){
          $(this).addClass("over");
        },function(){
          $(this).removeClass("over");
        }
      );

The patch is attached. I wonder, is there a more graceful solutions?
Thanks.

CommentFileSizeAuthor
#11 ie6_css_patchnopatch_170840.png16.34 KBChristefano-oldaccount
#9 nicemenus_ie6_170840.png57.89 KBChristefano-oldaccount
#8 nice_menus-iehoverclass-170840-8.patch1009 bytesadd1sun
#6 nice_menus-iehoverclass-170840-6.patch1.55 KBadd1sun
nice_menu_hover.patch550 bytesardas

Comments

jasonwhat’s picture

jasonwhat commented

Has this patch been applied yet?

add1sun’s picture

add1sun commented

Nope, it hasn't been applied yet. It needs to be tested first and I'd like a little more feedback on whether there is a better way to add this in. If folks familiar with JS could give a thumbs up or comments about it (as well as anyone doing general testing) that would move this in the right direction.

Crimson’s picture

Crimson commented

I haven't tested it but I'm very familiar with this code and use it all over my site. This works as described and is the most elegant way to get :hover to work for every browser.

add1sun’s picture

add1sun commented

Since we are already setting the over class in the JS, wouldn't it make more sense to just set the over class for all li and then separately add the other needed JS to the menuparent class? That is, why do the addClass twice?

ardas’s picture

ardas commented

The code I provided isn't ideal, this is only to give an idea of what is needed. I did the simplest fix which works but before it is included into the module I think it needs some work. Please provide a better solution.

add1sun’s picture

add1sun commented
StatusFileSize
new nice_menus-iehoverclass-170840-6.patch1.55 KB

Argh, OK, IE makes everything more difficult. We can't use just the over class because IE6 doesn't use multiple class selectors properly (i.e. only apply this style if the class is menuparent AND over) so we need a different name for the class anyway. Silly IE. Anyway, this patch simply does the above but names the class "ie-over" for every li in the menu and then the menuparents get an additional "over" class too. I'm not a huge fan of how that is all named by I don't want to mess up backwards compatibility for people that have already themed stuff based on the over class being the equivalent of menuparent. So there it is.

If I can get someone to review this and make sure it works as advertised, I'm ready to roll it in to the next release of Nice Menus.

add1sun’s picture

add1sun commented
Version: 6.x-2.x-dev » 5.x-1.0

Note this is against the Drupal 5 version and will go there and HEAD when committed.

add1sun’s picture

add1sun commented
StatusFileSize
new nice_menus-iehoverclass-170840-8.patch1009 bytes

Whoops the performance patch makes this fail on D5 branch. New patch.

Christefano-oldaccount’s picture

Christefano-oldaccount commented
StatusFileSize
new nicemenus_ie6_170840.png57.89 KB

You are braver than I am, add1sun. Here's the screenshot we discussed on IRC.

add1sun’s picture

add1sun commented
Status: Needs review » Needs work

Hm, darn, I can't reproduce that on IE6 or IE7 on Win XP. The iFrame is totally invisible on my sites.

Did you try this without the patch as well in Garland, on the same site? Someone else had this issue before so I'm wondering if this is not related to this patch per se.

Also could you maybe try changing the iframe CSS to see if it helps? Trying changing the width and height on the iframe (in nice_menus.css) to something like 40px:

ul.nice-menu li.menuparent ul iframe,
/* For Garland header. */
#header-region ul.nice-menu li.menuparent ul iframe {
        ...
	width: 40px;
	height: 40px;
}

Then put the nice menu in the left sidebar and go to the Date settings page (admin/settings/date-time) and make sure it still looks OK when you fly out over the select dropdowns.

Christefano-oldaccount’s picture

Christefano-oldaccount commented
StatusFileSize
new ie6_css_patchnopatch_170840.png16.34 KB

I think my involvement is accidentally derailing the issue. It's possible that I'm experiencing something completely different because changing the width and height to 40px seems to completely fix the iframe glitch in the last screenshot, whether nice_menus.js is patched with #8 or not.

  • Without the #8 patch and without changing the CSS, the glitch appeared only when the menu style was set to "down" (and it went away until the page was reloaded).
  • With the #8 patch and without changing the CSS, you get the attached screenshot. Again, the glitch shows up only when the menu style is "down" but this time it doesn't go away.

These tests are with IE 6.0.2800.1106 (IE6 SP1) running in a Windows 98 Wine bottle. I'd gladly test in a "real" Windows environment but my VMware image just blew up and I haven't yet reinstalled Windows.

add1sun’s picture

add1sun commented

OK, I've made a new issue about the iFrame thing and we can continue dissecting that over there. http://drupal.org/node/200990

add1sun’s picture

add1sun commented
Status: Needs work » Needs review

OK, since the iFrame thingy is being handled in another issue I'm marking this back for review. I'd *really* like to have this tested in a custom theme that is currently working to make sure it doesn't mess it up for any reason, preferably in both IE6 and IE7.

If someone could even do a little CSS to verify that IE is working properly with ie-over class that would be awesome. I have tested all of this locally (but not on a custom theme) and it all seemed to work as advertised.

add1sun’s picture

add1sun commented
Version: 5.x-1.0 » 6.x-1.x-dev

Moving this to 6.x-dev. I will backport to 5 once we verify this works properly. The latest patch *should* apply to the 6 branch but I haven't actually tested that.

Really need testers so if you want this in the module please test!

add1sun’s picture

add1sun commented
Status: Needs review » Fixed

Hm, since no one seems to be up for testing this from a patch I'm going to put it in the dev snapshot to see if folks will test that. Committed to 6 and HEAD.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

bdornbush’s picture

bdornbush commented

I read this posting while trying to fix an IE6 problem, which had to do with the colors of text and background, and I was able to fix it once I found the following:

"In IE6, the :hover pseudo-class applies only to hyperlinks (which is incorrect under the CSS 2 specification), whereas other browsers recognize the :hover pseudo-class on any rendered element, per the CSS 2 specification. This problem is fixed in IE7." from Beginning CSS by Richard York. (I highly recommend this book!!)

I just duplicated the CSS code that was at the "li" level also at the "li a" level, and my problem was fixed.

harking’s picture

harking commented
Status: Closed (fixed) » Active

I've noticed that the nice_menus.css file still uses "over" instead of "ie-over" for the class selector.

Is this on purpose?

harking’s picture

harking commented
Status: Active » Closed (fixed)

NM, I was confused in the double application of "over" and "ie-over". Maybe both are not needed?