← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~rharding/launchpad/productjs into lp:launchpad

 

Review: Approve

> #39: You are correct. The next step is to add in code to try to figure out how
> to work around adding information type as a feature flag so this isn't
> currently processed.

Thank you.

> #66 This was from the existing code. I just kept the regex into place. I'm not
> 100% sure on the best way, but that would be a follow up UX fix for submitting
> a project that might require some discussion/testing.

Ah, yes, I missed that.

> #266 I've attempted to keep with the Y.View api in YUI 3.5.1 as at some point
> this might be a View object vs just JS that runs on the page.
> 
> It doesn't involve renderUI and bindUI because those are specific to the
> Widget interface and this isn't a Widget. A Widget is a much smaller more
> reusable bit of JS/HTML combo while this is more a large View with events and
> code that occurs within it.

Ah, okay. I saw `render` and `bindUI` and thought `Widget`. 
 
> #291 Yea, I guess that not all node selections are done via ATTRS. I only
> pulled in the ones that were used in more than one place and noted that
> they're made ATTRS so that we don't redo the DOM lookup when it's not
> required. This is because I'm working on breaking apart the old code that one
> as one giant JS block into a series of calls I can work on splitting and
> testing as they break apart.

Ok, thanks.
-- 
https://code.launchpad.net/~rharding/launchpad/productjs/+merge/123803
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.


References