review: bug parade component patch

Colin Clark colinbdclark at gmail.com
Mon Dec 21 17:49:37 UTC 2009


Hi Andrea,

On 18-Dec-09, at 8:24 AM, Andrea Leutgoeb . wrote:

> Patch to be reviewed pls:
>
> http://issues.fluidproject.org/browse/FLUID-3305?focusedCommentId=16874&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_16874
>
> *Changes*
> - Jslinted parade.html & parade.js
> - New jslinted standalone version parade-standalone.html

Thanks for the patch!

I'm wondering if you can explain a bit more about your goals with the  
parade-standalone.html page? At first glance, I'm not sure if it's the  
best approach to duplicate all the markup and code from parade.html.  
Is it the case that you're trying to switch between loading data from  
the server and from a local test file?

If that's the case, you can probably use a common technique we use  
throughout Engage: check the URL at window.location to see if it's a  
file:// URL instead of a server-based URL. Here's an example of some  
sketch code in Engage for this task:

http://source.fluidproject.org/svn/fluid/engage/fluid-engage-core/trunk/framework/js/loadData.js

As for the rest of the patch, I'm wondering if you could address a few  
structural issues before running JSLint. I think this will make it  
easier to read, review, and lint:

1. Remove the unnecessary code from the script block in parade.js. It  
looks like Joan still has code in here that has been cut and pasted  
from a demo related to instantiating Inline Edits. No need to lint  
code that is old and unused.

2. Move the remaining code located in parade.html into the Parade.js  
file. This way, the component can be entirely self-contained and can  
be initialized and run with a single function call. I can help you  
with the specifics of

Here's a code review from Michelle that outlines a few of these issues:

http://old.nabble.com/Bug-Parade-Component-Review-td26504172.html#a26504172

I hope this helps, and please don't hesitate to ping us in the channel  
in the New Year if you want someone to walk you through the specifics  
of these fixes. We're happy to help.

Colin

---
Colin Clark
Technical Lead, Fluid Project
http://fluidproject.org




More information about the fluid-work mailing list