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

_______________________________________________________
fluid-work mailing list - [email protected]
To unsubscribe, change settings or access archives,
see http://fluidproject.org/mailman/listinfo/fluid-work

Reply via email to