Skip to content
This repository has been archived by the owner. It is now read-only.

Selectmenu: Add classes option #8309

Conversation

gabrielschulhof
Copy link

@gabrielschulhof gabrielschulhof commented Oct 17, 2015

No description provided.

@arschmitz
Copy link
Contributor

arschmitz commented Oct 18, 2015

@gabrielschulhof were you wanting a review of this

@gabrielschulhof gabrielschulhof force-pushed the 7731-selectmenu-classes-option branch 2 times, most recently from cd1e657 to e7f778d Compare Nov 10, 2015
@gabrielschulhof
Copy link
Author

gabrielschulhof commented Nov 10, 2015

@arschmitz I've made a few more changes and I could use a review although I'm fairly certain it's not ready to go yet, but it'll be good to have another set of eyes on it so I have a clear list of what remains to be done. TIA!

@@ -79,16 +94,24 @@ return $.widget( "mobile.selectmenu", $.mobile.selectmenu, {
}

if ( event.type === "vclick" ||
event.keyCode && ( event.keyCode === $.mobile.keyCode.ENTER || event.keyCode === $.mobile.keyCode.SPACE ) ) {
event.keyCode &&
( event.keyCode === $.mobile.keyCode.ENTER ||
Copy link
Contributor

@arschmitz arschmitz Nov 19, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use $.ui.keyCode this is deprecated

@arschmitz
Copy link
Contributor

arschmitz commented Dec 3, 2015

Looking good done for now

@gabrielschulhof gabrielschulhof force-pushed the 7731-selectmenu-classes-option branch from 15f9447 to 33a5648 Compare Jun 18, 2016
@gabrielschulhof
Copy link
Author

gabrielschulhof commented Jun 19, 2016

@arschmitz we have a load order race condition in kitchensink which seems to reliably manifest itself with core 1.9.1 in this PR. Aside from that it should be golden. I'll try to look into it whenever I get a chance.

@gabrielschulhof
Copy link
Author

gabrielschulhof commented Jun 19, 2016

@arschmitz nm, using a $.Deferred(). Lessee...

@gabrielschulhof
Copy link
Author

gabrielschulhof commented Jun 19, 2016

Yaaaay! Tests are now no worse than 1.5-dev!

@arschmitz
Copy link
Contributor

arschmitz commented Jun 22, 2016

👍

gabrielschulhof pushed a commit that referenced this pull request Jun 23, 2016
@apsdehal
Copy link
Contributor

apsdehal commented Jun 23, 2016

Awesome! @gabrielschulhof landed this in 1.5-dev.

@apsdehal apsdehal closed this Jun 23, 2016
arschmitz pushed a commit to arschmitz/jquery-mobile that referenced this pull request Jul 4, 2016
arschmitz pushed a commit that referenced this pull request Jul 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants