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
_______________________________________________________
fluid-work mailing list - [email protected]
To unsubscribe, change settings or access archives,
see http://fluidproject.org/mailman/listinfo/fluid-work
