Menubar #979

Closed
wants to merge 8 commits into
from

Projects

None yet

3 participants

@sgharms

Per the wiki: initial implementation of a vertical menubar. Will squash once reviewed.

@jzaefferer
jQuery Foundation member

Thanks @sgharms. I've started reviewing this. I've noticed a few issues on the updated demo page, but before get there, I have to get back the original feature request. The discussion that lead to adding the TODO item is so long ago, that I can't remember any of it. Maybe @scottgonzalez can remember something. What I'm currently wondering about: Why bother with a vertical menubar, when that is so similar to a inline menu? The only differences I see are styling (menubar looks different from menu) and activation (menubar can be configured to activate on click, instead of hover like menu always does). Considering the complexity this option adds (125 new LOC, leading to ~600 LOC in total) it seems questionable if that option really makes sense.

I'm not saying that we need to throw this away, I just want to make sure we add this for the right reasons.

As for the issues I noticed in the demo: The second menubar, the one that opens on hover, is broken. Moving the mouse from one item to the next causes the next item's menu to be displayed briefly, before it closes. The third menubar is just broken, it doesn't do anything on hover or click.

@scottgonzalez
jQuery Foundation member

ARIA requires menubars to be horizontal.

@sgharms

@jzaefferer: There's definitely still some warts on this, I didn't try all the permutations of the demo set. By the time I got the first one working I was kinda 👎 on going further with it. It seems you're of a similar position and that @scottgonzalez has a similar feeling on it (and may have an even better reason i.e. the ARIA specification).

I'm more interested in the ui-state-disabled` task at the moment anyway. I would not 😢 if this work were to be thrown away.

@jzaefferer
jQuery Foundation member

Sorry for wasting some time on this. But hey, at least we gave it a try.

I've updated the wiki page to reflect the decision not to support vertical menubars: There's an explicit "not supported" section in the functional spec. I've removed the TODO and the related "open issues being discussed". I've also updated the other TODOs and added details to the functional spec where useful for the TODOs. This now looks like a pretty clear roadmap.

For the wiki diff, see http://wiki.jqueryui.com/w/page-revisions/compare/38666403/Menubar?rev2=1368135764&rev1=1367867849

@jzaefferer jzaefferer closed this May 9, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment