Menu: ui-state-focus styling applies to submenus

Hi tasso85,

Thanks for taking the time to contribute to jQuery UI. Unfortunately it's very difficult to see what the issue is in your test cases because there's a whole lot of code involved. Can you reduce the test case to use the least amount of code necessary to show the bug? I'm not seeing issues in my trivial test case: http://jsfiddle.net/3fFTu/.

Hi,

The issue is that sub-menus items inherit from parent's ui-state-active style. So depending of the theme, this can lead to unexpected display.

For instance in the following fiddle, sub-menus items are bold: http://jsfiddle.net/sebfz1/rDT7T/

Thanks,

Sebastien.

@tj.vantoll: the issue is exactly the one explained by @sebfz1, that is, sub-menus items seems to inherit from parent's ui-state-active/ui-state-focus style (at least when they contain <a/> tags).

I described my issue in the original post itself:

• open my fiddle http://jsfiddle.net/ucF9M/1/
• open the hidden submenu
• hover on "Inserimento multiplo"

you will notice that the submenu has white text on a white background.

If you do the same on my other fiddle, which uses previous jQuery UI version, the submenu is rendered correctly.

See also your own, slighly modified, fiddle: http://jsfiddle.net/3fFTu/1/

Thanks, that helped a lot. I get the problem now, and see that it wasn't a problem in 1.10.4. This could be a tricky one to solve.

Replying to [comment:4 tj.vantoll]:

Thanks, that helped a lot. I get the problem now, and see that it wasn't a problem in 1.10.4. This could be a tricky one to solve.

This may sound a little naive, since I've never tinkered with jQuery UI source code I do not know it's internals, but, in version 1.10.4, the focus class gets applied only to the <a/> tags inside the <li/> element, while now it is applied to the whole <li/> element, can't you just make it behave as it did before?

Replying to [comment:5 tasso85]:

Replying to [comment:4 tj.vantoll]: > Thanks, that helped a lot. I get the problem now, and see that it wasn't a problem in 1.10.4. This could be a tricky one to solve. This may sound a little naive, since I've never tinkered with jQuery UI source code I do not know it's internals, but, in version 1.10.4, the focus class gets applied only to the <a/> tags inside the <li/> element, while now it is applied to the whole <li/> element, can't you just make it behave as it did before?

Well, it defeats the purpose of removing the anchor tags to add them back, but I think we need *some* element to workaround this problem. Our best bet is probably to wrap each menu item with a span internally and apply the class name to that span.

This is the smallest test case I can come up with: http://jsfiddle.net/tj_vantoll/3fFTu/2/. Now to work on fixing this.

In order to fix this, we need to add wrappers again. This time we won't require anchors, but we'll likely need to add a class to the wrapper in order to properly apply the styling and handle the upcoming classes option.

Unfortunately, since requiring the wrappers again is another breaking change, we'll need to wait until 1.12.0 to fix this.

Replying to [comment:8 scott.gonzalez]:

In order to fix this, we need to add wrappers again. This time we won't require anchors, but we'll likely need to add a class to the wrapper in order to properly apply the styling and handle the upcoming classes option. Unfortunately, since requiring the wrappers again is another breaking change, we'll need to wait until 1.12.0 to fix this.

Is there any chance to obtain some kind of patch/hotfix for this?

For example, I could create a new plugin which inherits from the menu plugin and hooks in the create method to put the wrappers dinamically, or use <a/> tags if present, but I fear my knowledge of jQuery UI internal is not good enough to make this patch by myself...

Replying to [comment:9 tasso85]:

Is there any chance to obtain some kind of patch/hotfix for this?

Everything we implement is public, so once it's implemented, you can just grab the copy from master. However, with that being said, this won't land in master until we're getting ready for 1.12.0. I'd like to make that happen a lot sooner than we planned because of this.

Replying to [comment:9 tasso85]:

>

Is there any chance to obtain some kind of patch/hotfix for this?

You can also workaround this with a little CSS that undoes any rules you don't want applied to submenus. For example http://jsfiddle.net/tj_vantoll/3fFTu/3/.

Replying to [comment:11 tj.vantoll]:

You can also workaround this with a little CSS that undoes any rules you don't want applied to submenus. For example http://jsfiddle.net/tj_vantoll/3fFTu/3/.

Unfortunately, you cannot. It inherits some things other than just font weight. And if you correct them all with custom css, the moment you change the theme, you must again modify your CSS for the new theme.

I suspect the wrappers were removed because of mobile version. But anyway, I don't see other solution but wrappers. A widget option for this would satisfy both wrapper heaters and lovers I think. Or, if there are submenus, a wrappers only for submenus can be made. Submenus are unusable this way.

...and again - I don't know why the "smoothness" theme was chosen for the demo sites. It is the worst theme to see a bugs so obvious in some other themes.

I'm working on a PR for this.

#10633 is a duplicate of this ticket.

PR: https://github.com/jquery/jquery-ui/pull/1342

Menu: Wrap menu items in a <div>

This avoids styling issues where ui-state-focus rules apply to submenus.

Fixes #10162

Closes gh-1342

Changeset: de2ef2a585447cf75fc8d4a734c12aa2bbd028c2