Some notes on Capture

Boyan Sheytanov boyan.sheytanov at asteasolutions.com
Sun Jan 17 20:04:56 UTC 2010


Hi Michelle!

Thanks for the feedback. Please see my comments inline below.

2010/1/14 Michelle D'Souza <michelle.dsouza at utoronto.ca>

> Hi Boyan,
>
> I've been taking a closer look at your work in Capture and I thought I'd
> send along a few notes that may help in improving the code.
>
> I noticed that you were able to remove the use of 'find' on line 89 which
> released the API restriction of the selector for the deleteButton. However,
> on line 182 the issue is reintroduced. After looking at it a bit longer, I'm
> feeling like the handling of the delete button is more complicated then it
> needs to be. Instead of using show and hide like I suggested, you could
> simply use CSS to deal with the styling. This should do the trick:
>
> .fl-reorderer-movable-default .flc-capture-button-delete {
>   display: none;
> }
>

I already thought about that - will implement it this way - will be much
simpler.


>
> I might be missing something here, but I think the actual click handling of
> the button could be handled by adding the click handler at the time of
> rendering using a jQuery decorator. At that time you should know which item
> you are rendering so your click handler would know which item to delete.
>

Well, I have done something really stupid here. Of course the binding should
not be performed in the onSelect listener but when the item is added to the
markup. Another thing that will simplify the code.


>
> It seems to me that we could find a better name for 'initialModel'. The
> word model implies the data for the component that is abstracted from the
> DOM where as in this case we are talking about a DOM element. Is that
> actually a thumb template perhaps? I'm really not good at naming so
> hopefully you'll find a better name.
>

I also had difficulties with making up the name. 'itemTemplate' is probably
good, but will try to think of something better.


>
> There is some repeated code in the 'if' and 'else' blocks starting on line
> 243 - perhaps there is a better way to factor this code. At the very least
> the 3 identical lines could be pulled out of the if and else blocks.
>

Oh, it seems these have come after introducing the serverOn option. It would
be pretty simple to move them after the block.


>
> I'll be looking at your code regularly so expect more of these messages as
> you progress.
>
> Hope this is useful,
>
> Michelle
>

Seems like another pair of eyes are very useful for spotting obvious
problems. These all are sensible and I will work on eliminating them. It
will be great to have more notes like this.

Thanks,

Boyan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://fluidproject.org/pipermail/fluid-work/attachments/20100117/d5904fcf/attachment.html>


More information about the fluid-work mailing list