WATableReport enhancement

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

WATableReport enhancement

Dan Winkler
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
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev

Seaside-Widgets-DanWinkler.19.mcz (64K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: WATableReport enhancement

Julian Fitzell-2
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 <[hidden email]> 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
> [hidden email]
> http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
>
>
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
Reply | Threaded
Open this post in threaded view
|

Re: 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 <[hidden email]> 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 <[hidden email]> 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
> [hidden email]
> http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
>
>
_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev


_______________________________________________
seaside-dev mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev