[Seaside-dev] WATableReport enhancement

[Dan Winkler]

Dear Julian,

Thanks for the excellent feedback!  It will come in handy if I want to
contribute something else in the future.

No, I wasn't aware that valueBlock could take two arguments and you're right
that doing it that way does give more flexibility than what I implemented.
 So I'd like to withdraw my proposed changes.

Thank you,

-- Dan

On Wed, Oct 6, 2010 at 6:26 PM, Julian Fitzell <jfitzell at gmail.com> wrote:

> Hi Dan,
>
> Thanks. I took a quick look at this. Are you aware that you can
> currently do the following?
>
> WAReportColumn new
>    valueBlock: [ :pet :html | html anchor url: pet url; with: pet petName
> ];
>    title: 'Name'
>
> At first glance, this seems to give more flexibility, but if you feel
> it doesn't meet your needs, I'm happy to look at it again.
>
> Since I did review the .mcz before I found that method, though, I'll
> pass on the feedback I had noted:
>
>  * We don't use #ifNil:ifNotNil: for portability reasons. use "isNil
> ifTrue: [] ifFalse: []"
>  * There are a couple of methods in your version that have been
> changed but have no actual changes in them. Please make sure to check
> the changes before committing your version and revert any methods
> needed to remove these cases (they lead to conflicts down the road and
> confusion about who wrote the method)
>  * Our coding conventions put spaces around the contents of blocks and
> cascaded method sends each on their own lines.
>
> So this:
>
> (aColumn anchorBlock) ifNil: [html text: text]
>        ifNotNil: [html anchor url: (aColumn anchorForRow: aRow); with:
> text]
>
> Should really be more like:
>
> aColumn anchorBlock isNil
>        ifTrue: [ html text: text ]
>        ifFalse: [ html anchor
>                url: (aColumn anchorForRow: aRow);
>                with: text ]
>
> None of these are the end of the world (i.e. we can fix them as we
> merge) but the easier you can make it for us, the more we'll love you.
> :)
>
> Julian
>
> On Tue, Oct 5, 2010 at 2:28 PM, Dan Winkler <dan.winkler at gmail.com> wrote:
> > Attached is my enhancement to WATableReport which allows items in the
> table
> > to link to other web pages.  Previously it was only possible to attach a
> > Smalltalk callback to an item, not a regular HTML anchor to another web
> > page.
> > Example usage:
> >
> > petsTable := WATableReport new.
> > petsTable
> > columns:
> > ((OrderedCollection new)
> > add:
> > ((WAReportColumn new)
> > valueBlock: [ :pet | pet petName ];
> > anchorBlock: [ :pet | pet url ];
> > title: 'Name');
> > add: (WAReportColumn selector: #age title: 'Age');
> > add: (WAReportColumn selector: #birthDate title: 'Birth Date');
> > add: (WAReportColumn selector: #gender title: 'Gender');
> > add: (WAReportColumn selector: #breed title: 'Breed');
> > add: (WAReportColumn selector: #petNumber title: 'Number');
> > yourself)
> > _______________________________________________
> > seaside-dev mailing list
> > seaside-dev at lists.squeakfoundation.org
> > http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
> >
> >
> _______________________________________________
> seaside-dev mailing list
> seaside-dev at lists.squeakfoundation.org
> http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/seaside-dev/attachments/20101006/7a690a14/attachment.htm