Selectmenu: does not preserve tabindex

[Simen Bekkhus]

I'm not able to provide a fiddle for this, so I'll just reference the code that has what I think the problem is.

When you call the refresh-method of selectmenu, the _refreshMenu method always sets the disabled option, based on whether or not the actual element is disabled. ​See this line of code. Then, if the element is disabled, the tabindex is set to -1. If the element is *not* disabled, it's set to 0. ​See this line of code.

That last thing I believe is wrong. tabindex shouldn't be set to 0 if the element is not disabled, it should be set to whatever the tabindex of the backing select-element is, and if that's not set, then it should be set to 0.

I created a ​pull request with a proposed fix for this.

[tj.vantoll]

Actually the problem seems bigger than just refresh(). If I assign a tabindex to the <select> it is not applied to the button, before even calling refresh(): ​http://jsfiddle.net/tj_vantoll/5jqjfzvt/.

SimenB > Could you expand your PR to address this as well?

[Scott González]

It's generally a bad idea to set a tab index greater than 0. I'd prefer to keep the refresh logic the way it is.

[Scott González]

Here's a recent W3C discussion related to tabindex greater than 0: ​http://lists.w3.org/Archives/Public/public-html/2014Oct/0027.html

[Simen Bekkhus]

While I appreciate (and totally agree) with your reasoning, I still think it's flawed.

The intro for Selectmenu on your site ​here is

Duplicates and extends the functionality of a native HTML select element to overcome the limitations of the native control.

If you ignore tabindex, you no longer duplicate the functionality of native HTML select.

Even though W3C might in the future discourage use of tabindex, I still think Selectmenu should mirror whatever the markup for the element is. If someone has set a tabindex on the select-element, they've probably put it on surrounding elements as well. Then when using Selectmenu, the tabbing gets messed up.

It seems weird that Selectmenu should be so opinionated. We use it (mainly) to be able to style our selects, not for it to impose a suggestion still in draft form from W3C that removes (maybe wrong word) functionality that's been in the browser forever.

How about printing a warning of some sort in the console instead, maybe when using jQuery Migrate or something?

Again, I completely agree with the reasoning, but I'm currently working on an application with thousands of lines of code, a lot of it (infested) with tabindexes, and for jQuery UI Selectmenu to require quite a massive refactoring before working properly with regards to tabindex (without modifying/extending the source, which we currently do) seems wrong to me.

[Scott González]

jQuery Migrate is for jQuery core, not jQuery UI or any other jQuery plugins. It also prints messages about deprecated functionality, while this was never something that worked in jQuery UI.

[Simen Bekkhus]

Ah, OK, I thought it was for jQuery in general.

We had tabbing working, and with the change in the PR, refresh worked as well.

Would it be possible to take a look at why tabindex isn't working properly, instead of removing support for it altogether?

[Simen Bekkhus]

If I expand the PR to cover the initialization as well, can that make it in?

The problem is with tabindex || this.options.disabled ? -1 : 0. Pseudo code: if tabindex or disabled, then -1, otherwise zero.

Wrapping the right side of || in parenthesis fixes it. See ​fiddle.

I override _drawMenu, otherwise it's the same as tj.vantoll's fiddle.

[tj.vantoll]

Here's an extension that implements the tabindex mirroring you're looking for:

$.widget( "ui.selectmenu", $.ui.selectmenu, {
    _create: function() {
        this._super();
        this._setTabIndex();
    },
    _setTabIndex: function() {
        this.button.attr( "tabindex",
            this.options.disabled ? -1 :
            this.element.attr( "tabindex" ) || 0 );
    },
    _setOption: function( key, value ) {
        this._super( key, value );
        if ( key === "disabled" ) {
            this._setTabIndex();
        }
    }
});

Demo: ​http://jsfiddle.net/tj_vantoll/b3fuezsb/

If others are interested in this functionality being natively incorporated they can comment here or vote on this issue. For now we're going to leave the behavior as is, and if you need custom tabindex attributes use the extension.

[Simen Bekkhus]

I still think it's inherently wrong not to mimic the native select as closely as possible out of the box, but this works for me. Thank you for the short and sweet snippet!

EDIT: Maybe you could add this to the examples, like with ​autocomplete with categoriers?