Selectmenu: Code review

[fnagel]

Request for code review (in accordance with Jörn).

There some open issues being discussed, see the Wiki:
http://wiki.jqueryui.com/w/page/12138056/Selectmenu

[gnarf]

_newElementEvents?

[fnagel]

Named that way because the events are bound to this.newelement. Variable names (this.list, this.newelement, etc.) are taken over from the old branch. The naming of the vars should be discussed anyway :-)

[fnagel]

See here: #492 (comment)

[gnarf]

I'm seeing a lot of uses of var that = this; where its not event needed. We are trying to avoid this wherever possible now...

Should probably just replace with this in most cases, as we clean up most of these, we can look at a better workaround for any cases left over.

[jzaefferer]

Can just use this within a _bind callback. Should remove need for that inside _create completely.

[jzaefferer]

Should also rename newelement to something like button or trigger. The latter would match what we have in Popup.

[fnagel]

1. Ahh I did get that wrong. Will be changed soon.
2. Agree, button seems legit. Should we change this.list to this.menu? I will change the wrapper var too.

[jzaefferer]

Sounds good.

[fnagel]

Done, pushed soon.

[jzaefferer]

Nice: Showing 13 changed files with 1,000 additions and 0 deletions.

[fnagel]

Let's clean up the visual tests. They are useful while prototyping, but need to be replaced by unit tests. Units are all there, so let's remove most of the visual tests, maybe merge one from each into a composite one.

Hey @jzaefferer, I know you want to get rid of the visual tests but they are very useful for testing issues and while developing. I've merged some again. Only three files left, hope thats ok.

[jzaefferer]

Looking pretty good. Still need to ui-front and icons updates, pinged Scott about that. Also looking for @hanshillen to help with the aria tweaks. Once that is done, I'll rebase my branch and merge it in here.

[scottgonzalez]

.ui-front implementation in autocomplete: 80e46c9

[fnagel]

Thanks @scottgonzalez, I will take a look asap. For now, its "The Hobbit" time :-)

[scottgonzalez]

no XML; use "<li>"

[fnagel]

Fixed.

[jzaefferer]

Closing this PR since notification emails mostly lead to "outdated diff", so nowhere and its generally just crazy long. Will create a new one from the same branch.