filter widget - first commit #6145
Conversation
|
Thanks @frequent ill look at this tonight. |
|
Awesome! Thanks a lot @frequent ! |
|
@uGoMobi, @arschmitz |
|
@frequent We use jshint. Rules are in https://github.com/jquery/jquery-mobile/blob/master/js/.jshintrc. At the command line just type: |
| define( [ "jquery", "./forms/textinput" ], function( jQuery ) { | ||
| //>>excludeEnd("jqmBuildExclude"); | ||
| (function( $, undefined ) { | ||
| "use strict"; |
There was a problem hiding this comment.
No "use strict" in client code.
| }, | ||
|
|
||
| _onKeyUp: function() { | ||
| var self = this, |
There was a problem hiding this comment.
no need to use self here just use this all the way down. self is generally not a needed concept in widgets since the this reference is maintained for events and setTimeout by using widget methods.
|
Shouldn't you add a demo that shows how to use this filterbar together with listview? |
|
It really helps to reference issues in commit messages and PRs. This PR fixes #5629 |
|
@jzaefferer, @arschmitz |
|
@frequent I meant demos inside this repo. At least a visual test. Something that other contributors will see in the future, once this PR is closed. |
|
@jzaefferer And pardon for the missed fix mention. |
| var o = this.options, | ||
| wrapper = document.getElementById( "ui-filter-" + this.uuid ); | ||
|
|
||
| if ( o.enhanced === false ) { |
There was a problem hiding this comment.
Just use if ( !o.enhanced ) {
|
Regarding demos: Yes, exactly. |
|
@uGoMobi, @arschmitz: If you want to help out:
|
@uGoMobi, @arschmitz
Through with testing. Everything passes on my machine (tested functions on FF, Chrome, IE8 so far Qunit on FF, Chrome)
Some notes:
self._getFilterableItems()is not defined. Still chasing this, but makes no sense to me, as it's defined in the function preceding the function that calls it.setTimeoutquite a bit in the tests. If that's not the way to go, let me know and I will amend.ul, table, div, controlgroup, select, p, span. Any others I should add and all else, please also let me know.