Code review comment for lp:~salgado/lazr-js/bug-482235

Revision history for this message
Björn Tillenius (bjornt) wrote :

On Fri, Dec 18, 2009 at 06:30:31PM -0000, Guilherme Salgado wrote:
> === modified file 'src-js/lazrjs/picker/picker.js'
> --- src-js/lazrjs/picker/picker.js 2009-11-24 00:09:41 +0000
> +++ src-js/lazrjs/picker/picker.js 2009-12-18 18:30:30 +0000
> @@ -863,10 +863,39 @@
> }
> };
>
> +
> +function TextFieldPickerPlugin(config) {
> + TextFieldPickerPlugin.superclass.constructor.apply(this, arguments);
> +}
> +
> +TextFieldPickerPlugin.NAME = 'TextFieldPickerPlugin';
> +TextFieldPickerPlugin.NS = 'txtpicker';
> +
> +Y.extend(TextFieldPickerPlugin, Y.Plugin.Base, {
> + initializer: function(config) {
> + // Y.one() uses CSS3 selectors which don't work with ids
> + // containing a period, so we have to use Y.DOM.byId().
> + // XXX: For that reason we need callsites to provide an element ID
> + // rather than a selector here.
> + var input = Y.DOM.byId(config.input_id);

This is not quite true. You can use CSS3 selectors if you use attribute
selectors. For example:

    var input = Y.one('[id="some.id"]');

The rest looks good in general, it looks nice. Although I'm missing some
documenation. Do you plan to add some?

--
Björn Tillenius | https://launchpad.net/~bjornt

« Back to merge proposal