right. i agree. that's why it's Page and not Class to detemine identity.
but Class constructor is the 90% case. i think you're right though that
BookmarkablePageLink handles that case. it auto-disables and stores
less state in the page map, right?
and if you're going to use something beyond class for identity, or you're
not using bookmarkablepagelinks then you're subclassing link. is that
right? like in a case like this:
editCell.add(new PageLink("edit", new IPageLink()
{
public Page getPage()
{
return new EditDvdDetails(getModel().getNestedModel());
}
public Class getPageIdentity()
{
return EditDvdDetails.class;
}
})
i'd have to write that linksTo() method you suggested and the getPage would
be a slightly longer onClick() method.
what i like about your way is that it eliminates a whole concept and
probably
a lot of confusion. the page identity concept and the interface for delayed
page
creation go away. the expense is that i have to write bigger onClick and
linksTo
methods. that's easier for newbies to understand though...
shoot. did you just convince me? i hate losing arguments to you, igor.
;-)
if bookmarkablepagelink and link really do provide everything i need to
keep auto-link-disabling easy, i guess we ought to remove PageLink and
IPageLink and rename BookmarkablePageLink to PageLink as you suggest.
but only in the 2.0 tree where people are on the bleeding edge.
igor.vaynberg wrote:
>
> you are assuming that identity is only defined by the class. but what if
> pageparameters/models are also involved?
>
> then that code has to go somewhere anyways
>
> -igor
>
>
> On 2/25/07, Jonathan Locke <[EMAIL PROTECTED]> wrote:
>>
>>
>>
>> let's just take a step back and ask: how can we make this less work?
>>
>> could new AutoDisableLink<T> work or does type erasure wipe out
>> that possibility?
>>
>> i mean ideally we could both have what we want here.
>>
>>
>> Jonathan Locke wrote:
>> >
>> >
>> > you meant page.getClass().equals(MyAccountPage.class)
>> > that's quite a mouthful to repeat. it makes something you
>> > don't care about but which i do all the time hard. just
>> > because it's more minimal doesn't make it better.
>> >
>> > i want more abbreviated support for auto-disabling of links
>> > in the core. extracting a subclass of Link like AutoDisableLink
>> > would be fine. but i don't really want to write the code below
>> > by default and i think other users deserve the benefit of this
>> > behavior by default too. it's how wicket makes link menus trivial
>> > and users should not have to go hunting on the wiki to figure
>> > it out.
>> >
>> >
>> > igor.vaynberg wrote:
>> >>
>> >> add( new link("foo") {
>> >> onclick() { setresponsepage(new MyAccountPage()); }
>> >> boolean linksto(Page page) { return
>> >> page.class.equals(MyAccountPage.class);
>> >> }
>> >> }
>> >>
>> >> simple as that and the bookmarkablepagelink already also does this
>> >>
>> >> -igor
>> >>
>> >>
>> >> On 2/25/07, Jonathan Locke <[EMAIL PROTECTED]> wrote:
>> >>>
>> >>>
>> >>>
>> >>> i don't think you really listened to me. the part i want is not the
>> >>> delayed
>> >>> creation of the page. you can get that by subclassing link. what i
>> >>> want
>> >>> for myself and all wicket users is the /identity/ part. without that
>> >>> wicket
>> >>> cannot disable self-referential links automatically. this makes
>> menuing
>> >>> with links a hassle. if you can explain how i can do delayed linking
>> >>> but
>> >>> still
>> >>> have wicket deal with disabling links to their containing page
>> >>> automatically,
>> >>> i'm fine with that. if not, i'm very much -1 on it.
>> >>>
>> >>>
>> >>> igor.vaynberg wrote:
>> >>> >
>> >>> > why not let us remove it, and you copy it into your project
>> >>> >
>> >>> > -igor
>> >>> >
>> >>> >
>> >>> > On 2/25/07, Jonathan Locke <[EMAIL PROTECTED]> wrote:
>> >>> >>
>> >>> >>
>> >>> >>
>> >>> >> i don't want to gang up on martijn here, but that's what i meant.
>> >>> >> i also think we should refactor pagelink with the remaining two
>> >>> >> constructors to:
>> >>> >>
>> >>> >> pageclasslink (the one with the class contructor)
>> >>> >>
>> >>> >> and
>> >>> >>
>> >>> >> delayedpagelink (the one with the ipagelink constructor)
>> >>> >>
>> >>> >> this way there is no class name pagelink (which is dangerous
>> because
>> >>> >> it's such an obvious name). this situation will make it very
>> obvious
>> >>> >> what users ought to do (now Link is the most obvious class) and
>> >>> >> pageclasslink
>> >>> >> and delayedpagelink will both be slightly more efficient since
>> they
>> >>> won't
>> >>> >> share an unused field. then users will have a choice between:
>> >>> >>
>> >>> >> link
>> >>> >> pageclasslink
>> >>> >> bookmarkablepagelink
>> >>> >> delayedpagelink
>> >>> >>
>> >>> >> that seems like a very good situation.
>> >>> >>
>> >>> >> if you're still unhappy, martijn, maybe we could keep the page
>> >>> >> constructor
>> >>> >> in a deprecated class like pagereferencelink. that would help
>> guide
>> >>> >> users
>> >>> >> away from the class, but it would still be there for backwards
>> >>> compat.
>> >>> >>
>> >>> >> jon
>> >>> >>
>> >>> >>
>> >>> >> Eelco Hillenius wrote:
>> >>> >> >
>> >>> >> >> Very convenient to hold a vote when I'm not around...
>> >>> >> >
>> >>> >> > It wasn't concluded yet. And the vote wasn't started because of
>> >>> your
>> >>> >> > absence, but because of a message to the list.
>> >>> >> >
>> >>> >> > Martijn, the tricky situation here is that everyone on the team
>> -
>> >>> and
>> >>> >> > I'm pretty sure including the ones who didn't vote - is very
>> much
>> >>> >> > against having this constructor. VERY much. It leads to a
>> dangerous
>> >>> >> > situation and as a message earlier this week showed, people ARE
>> >>> using
>> >>> >> > it the wrong way. God knows how many. This is not just a matter
>> of
>> >>> not
>> >>> >> > reading the docs for people, this is a matter of the API guiding
>> in
>> >>> >> > the wrong direction. Also, if documentation was enough, why
>> think
>> >>> >> > about API design in the first place? Also, comparing with
>> methods
>> >>> that
>> >>> >> > have the 'do not use this' warnings in them is just plain bs, as
>> we
>> >>> >> > have such 'constructs' because there was alternative to them,
>> >>> mainly
>> >>> >> > because Java doesn't have a friends construction.
>> >>> >> >
>> >>> >> >> -1 on removing the constructor. Just as Jonathan...
>> >>> >> >
>> >>> >> > Great, another veto. Jonathan was against removing the class
>> >>> >> > altogether (which I am personally +1 for, but not that strongly
>> to
>> >>> >> > make a lot of trouble). He stated he uses the IPageLink
>> constructor
>> >>> a
>> >>> >> > lot, while this vote is for removing the Page constructor.
>> >>> >> >
>> >>> >> > The only alternative I can see here is to leave the constructor
>> in
>> >>> -
>> >>> a
>> >>> >> > half baked solution which I would hate nevertheless - but put a
>> >>> >> > @deprecated tag with a big fat warning in it.
>> >>> >> >
>> >>> >> > Regards,
>> >>> >> >
>> >>> >> > Eelco
>> >>> >> >
>> >>> >> >
>> >>> >>
>> >>> >> --
>> >>> >> View this message in context:
>> >>> >>
>> >>>
>> http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9145872
>> >>> >> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>> >>> >>
>> >>> >>
>> >>> >
>> >>> >
>> >>>
>> >>> --
>> >>> View this message in context:
>> >>>
>> http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9147112
>> >>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>> >>>
>> >>>
>> >>
>> >>
>> >
>> >
>>
>> --
>> View this message in context:
>> http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9149820
>> Sent from the Wicket - Dev mailing list archive at Nabble.com.
>>
>>
>
>
--
View this message in context:
http://www.nabble.com/VOTE%3A-remove-PageLink%28String%2CPage%29-constructor-tf3274259.html#a9150350
Sent from the Wicket - Dev mailing list archive at Nabble.com.
