Selectmenu: Fix selecting options following hidden ones #2144
Conversation
| .filter( function( item ) { | ||
| return $( item ).is( ":visible" ); |
There was a problem hiding this comment.
I had to change the test as the options are now back in the DOM, just hidden.
Change a2b25ef made options with the `hidden` attribute skipped when rendering. However, that makes indexes misaligned with native options as hidden ones maintain their index values. Instead, don't skip hidden options but add the `hidden` attribute to the respective jQuery UI elements as well. Fixes jquerygh-2082 Ref a2b25ef
mgol
force-pushed
the
selectmenu-hidden-gh-2082
branch
from
c9d3147
to
ce06e9c
Compare
| this._setText( wrapper, item.label ); | ||
|
|
||
| if ( item.hidden ) { | ||
| li.prop( "hidden", true ); |
There was a problem hiding this comment.
I'm not sure about adding hidden prop here. We don't do that anywhere else. But it seems there is no other usage of the hidden attribute, so there is nothing to compare it to directly.
What about using "aria-hidden" attribute and "ui-helper-hidden" CSS class?
There was a problem hiding this comment.
@fnagel Sure, I can change the code. The behavior in IE 9 & 10 will be different but perhaps if the hidden attribute is there, it's expected that we should hide it even if the browser doesn't support it natively.
One thing - I see the ui-helper-hidden class definition in CSS but other than that, I don't see it anywhere in code; only the ui-helper-hidden-accessible one seems to be used. Am I missing anything?
There was a problem hiding this comment.
I'm not sure about my proposed change either :-D Just wanted to point out our options and get your opinion on it.
You are right, ui-helper-hidden is not used in JS. And ui-helper-hidden-accessible seems not correct here.
Maybe its best to use it as is it is (with the hidden property) and let the client (or its assistive technology) decide what to do.
There was a problem hiding this comment.
In that case, I think I'd leave it at hidden. @fnagel do you have other concerns? If not, could you approve?
Change a2b25ef made options with the
hiddenattribute skipped when rendering. However, that makes indexes misaligned with native options as hidden ones maintain their index values. Instead, don't skip hidden options but add thehiddenattribute to the respective jQuery UI elements as well.Fixes gh-2082
Ref a2b25ef