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

Revision history for this message
Māris Fogels (mars) wrote :

Hi Salgado,

Your changes look good. I have some comments about the implementation.

- You should use Y.one() instead of Y.get()
- Are you comfortable using a private member reference to get the raw DOM node (node._node)? There is a proper API function for that: Y.Node.getDOMNode().

- Just a comment: not in your code, but in the picker example page, I do not like the way the save event callback functions use array references and class-static variables to access the event details. They should be using objects. Do not worry about fixing it. The picker will have to be cleaned up and refactored at some point in the future.

Looks good. With changing Y.get() to Y.one(), r=mars.

Maris

review: Approve

« Back to merge proposal