JIRA[FLUID-3305] > bug parade component > request for early review

[Colin Clark]

Hi Joan,

I'm really excited to see your code. Very nice work!

I know your component is still really early, so don't worry too much  
about these comments. You can work on these issues as your component  
evolves. Here are some suggestions for you:

* In your paradeProcessItems() function, I don't think you need to  
create a custom clone function for your objects--fluid.copy() will  
provide you with a deep copy for many types of JavaScript objects.

* The paradeProcessItems() function contains some repetitive logic.  
For each item in the feed, you're finding it by name, getting its  
text, and trimming it. Although it's only one line of code, you would  
probably benefit from separating that logic into a function like this:
	valueForTagName = function (tagName) {
		$.trim(item.find(tagName).text());
	}

* In all your Ajax functions, I can't quite figure out why you're  
executing code in timeouts. Unless I'm missing something, this  
shouldn't be necessary. If you do need to use timeouts, use the  
version that explicitly takes a function rather than a block of code  
to be eval'ed.

• I think your render() function will be substantially improved by  
using the Renderer instead of emitting markup in code. We try to  
always avoid emitting markup in code so that users can more easily  
adapt and customize the markup. When some components do emit a trivial  
amount of markup in code (for example, Inline Edit), we try to ensure  
that this code is parameterized as a function in the options so it can  
be overridden by our users. But I think in the case of the Bug Parade  
component, the Renderer is the right tool for the job. At the moment,  
it's got a bit of a steep learning curve, but I think you can handle  
it. :)

• I think you could probably express your code in a more event-driven  
fashion. So, there are clearly a few events here, particularly related  
to when data is retrieved by the server. Perhaps these would make  
sense as events for your component? Take a look at the Flutter example  
for some inspiration.

• Jacob updated the version number of Infusion to fluid_1_2 in trunk,  
so I had to update your code to get it to run without errors. I also  
had to remove references to the inlineEdit demo code you had in there.  
I still can't get it to do anything, though. I must be missing  
something. Any suggestions?

• A couple of style/tidiness issues:
	- I don't think you need to prefix each of your functions with  
"parade." Because you've got them inside a closure, they'll be private  
to everyone. You can name them however you like	
	- There's extra code up at the top of your file that looks like it  
came from cutting and pasting a demo--inlineRichTextEditSetup(). It  
should be removed.
	- When you instantiate your component in the HTML init block, you  
pass it an empty object, then in separate step you register the event  
listener. You can do all that in one step like this:

         var parade = fluid.paradeComponent($(".fl-parade-container"), {
		events: {
			afterRender: function () {
             		alert("component rendered");
         	}
	   });

	- You'll probably want to run your code through JSLint to ensure that  
it's well-written and meets our code conventions. More information  
about JSLint is available on this page:
http://wiki.fluidproject.org/display/fluid/Coding+and+Commit+Standards

Once again, really great work. I look forward to seeing your progress  
on this component, and I hope this advice is helpful and constructive.

Colin

On 20-Oct-09, at 1:50 PM, Joan Garcia Vila wrote:

> http://issues.fluidproject.org/browse/FLUID-3305
>
> Hi all.
>
> This a request for you to let me know what needs to be fixed,  
> improved or whatever.
>
> I feel glad that the component only takes 3 lines in the html page.
> Very simple to use.
>
> I think that I'm starting to see the power and flexibility of the  
> infusion framework.
>
> cheers,
> joan.
> :-)

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