-
Notifications
You must be signed in to change notification settings - Fork 264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
datalist polyfills review #18
Comments
Thanks for the research! Couple notes on "rd" (Relevant Dropdowns). My intention is to leave feature detection using Modernizr. Modernizr has beefed up their detection for datalists (Modernizr.input.list) so that browser sniffing bit can be removed. (You were a part of that discussion: Modernizr/Modernizr#146) For option parsing, I'm pretty sure Respnsive Dropdowns supports your #1, #3, and #4, but I just opened your demo of it in IE 7 and it supported #2 as well, so maybe it's just IE 6 that can't handle #2. I personally think that's a fine level of support. For performance, Responsive Dropdowns uses the document fragment thing already. Maybe more can fit into the document fragment before adding? There is a couple of other small things too like using offset and using mousedown. That's all fine with me, just shoot over a pull request and I'm happy to take a look and merge it. |
No, problem. I think, rd datalist could be a really good markup polyfill. About the parsing problem.In fact, it renders all examples, but in case of 3 and 4 the label (explicit/implicit) has to be shown to the user. document fragementYes, I saw that you are using this technique. But I mean the following line, wich means, that you are creating one li-dom-object per option. If you would concat those li's to a string including the parent ul and simply use jQuery to create the DOM. jQuery would use one DOM-operation to create all li-elements at once. |
This is a short review of the current recommended datalist polyfill's. I think, those issues has to be fixed, before these scripts should be recommended.
ping @chriscoyier and @miketaylr
Additionally, I have created 3 fiddles (those two polyfills + webshims implementation) , to simply test those implementations (http://jsfiddle.net/trixta/3Yu9X/).
feature detection:
rd's implementation:
The main code of rd's implementation has no feature detection. Instead the demos are using the following feature detection:
While there is no problem using browser sniffing against past versions, it's not possible to sniff against future versions. This code breaks as soon as webkit has implemented the datalist element!!!
Use the following featuredetection:
To foolproof the script. The script itself has to use feature detection, so that a developer does not has to wrap his code invocation...
Note: This detection will be fixed with next Modernizr update.
options removed (X-browser???)
In HTML4 options are removed, if they are not a child of select or optgroup.
rd's demo
The demo does not use a select element and does not shiv the datalist-element. All options are removed, so that the demo doesn't even work with IE9. Please fix!!!!
mt's demo
This implementation adds a select element using conditional comments for IE. Due to the fact, that all older "HTML4-browser" remove option elements and not only IEs, it is recommended, that the demo should always wrap the options in a select element!!!
option parsing:
There are 4 ways to declare value/label of an option element:
At least 1 and 4 has to be supported.
rd's implementation
rd's implementation supports only 1 and 2.
mt's implementation
Is dependent from browser. Only 4. option syntax can be used with similiar results in all browsers. Use $.fn.val to also support first option syntax in IE7.
datalist[-overlay] placement
rd's implementation
rd's implementation appends the list to the body and uses $.fn.position to calculate the position of the input element. This is wrong and fails in a lot of cases. This fails as soon as the input element has an offsetParent (position: relative!!!), which is not html / body.
To make this right, $.fn.offset has to be used.
mt's implementation
This implementation is extremley obtrusiv and also fails in many cases. mt's implementation wraps the input element and the generated list inside of a div with relative position to place the datalist. This fails, in many ways, because
Use same technique as rd's implementation, but use offset instead of position.
user interaction
mt's implementation
no keyboard interaction, no list filter, no support for long datalists
rd's implementation
performance
mt's and rd's implementation [too much DOM operations]
Both implementations are creating one (rd) or three (mt) new elements per option. While rd's implementation should be fast enough in webkit, it's a really bad technique for creating a list of elements in IE7 and IE8. mt's implementation should be too slow in all browsers.
Instead of creating new elements per option. The implementations should create a string and let jQuery create the DOM at once using the fast DocumentFragment technique.
rd's implementation [event delegation]
use event delegation
mt's and rd's implementation [be lazy]
you don't see a list initially, only if a user interacts with the input, so be more lazy!
rd's implementation [resize event]
rd's implementation is realigning all datalists on the resize event even if the datalist isn't visible. The eventhandler doesn't use debouncing or throttling.
The text was updated successfully, but these errors were encountered: