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

Revision history for this message
Guilherme Salgado (salgado) wrote :

Hi Bjorn,

On Mon, 2009-12-21 at 12:12 +0000, Björn Tillenius wrote:
> On Fri, Dec 18, 2009 at 06:30:31PM -0000, Guilherme Salgado wrote:
[...]
> > +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"]');

I didn't know that I could use attribute selectors to workaround that,
and the comment was just copied from the LP code that does what is now
done by the plugin.

I've changed it the plugin to expect any CSS selector, instead of an ID,
added some docs and an example.

        http://paste.ubuntu.com/344218/
        Example: http://paste.ubuntu.com/344220/

And I've also moved the Picker from the Y namespace into the Y.lazr one,
after checking with Maris that that's where it belongs.

        http://paste.ubuntu.com/344216/

How does it look now?

--
Guilherme Salgado <email address hidden>

« Back to merge proposal