On 3/19/07, Frédéric Bertin <[EMAIL PROTECTED]> wrote:
However, from a user point of view, I find it a little bit complicated
and would find really nice to have a higher level method, like
setAjaxRefresheable(true|false),
we should keep the term "ajax" out of the method name. it needs to be more
abstract.
which takes care of
tuning all the setOutput* stuff so that a component is *always*
refresheable in ajax, even if it is initially setVisible(false), or
anything else.
As a user, I want to refresh my component with Ajax. I don't care if, to
achieve that, Wicket needs a tag, an id, or anything else.
which is what i proposed below in my code snippet
-igor
Does it make sense?
Fred
Igor Vaynberg wrote:
> im not a big fan of having an application-wide setting for this. it will
> make it harder to integrate with 3rd party components that werent
written
> with the setting in mind because it pretty much gives you an option of
> ignoring it by assuming a default.
>
> better make it explicit, and it doesnt add any extra work. instead of
> calling setoutputmarkupid(true) just call setoutputmarkuptag() or
> maybe even
> call it setoutputmarkuptagandid()
>
> -igor
>
>
> On 3/19/07, Matej Knopp <[EMAIL PROTECTED]> wrote:
>>
>> Well, I'm also -1 for having this behavior off for default.
>> But, i think it can be quite useful for couple of cases, therefore i
>> think there should be support for that.
>>
>> I think what we need is an application setting, for default behavior,
>> and then a getter for each component.
>> e.g. boolean Component.getRenderEmptyTagOnNotVisible()
>> that would by default return the application settings value.
>>
>> This would cost us nothing, as there is not even flag needed for it.
>> And would be versatile enough, to only enable the behavior on
>> components where it really make sense and doesn't break anything. But
>> this doesn't mean i couldn't live with a component flag of course :)
>>
>> -Matej
>>
>> On 3/19/07, Igor Vaynberg <[EMAIL PROTECTED]> wrote:
>> > what you need is a different method. instead of
>> setoutputmarkupid(true)
>> you
>> > need something that will do
>> >
>> > setoutputmarkuptag(boolean b) {
>> > if (b) {
>> > setoutputmarkupid(true);
>> > setflag(outputmarkuptag,true);
>> > } else {
>> > setflag(outputmarkuptag,false);
>> > //not sure if we should undo setoutputmarkupid here
>> > }
>> > }
>> >
>> > now this will solve the problems outlined so far
>> >
>> > btw the latest patch attached to the issue is bad because it executes
>> > behaviors which should not happen.
>> >
>> > what i am worried about are javascript libraries and css. does css 3
>> have
>> > support for odd/even selectors? that will break. this will even break
>> css 2
>> > first child selector if that happens to be invisible because i dont
>> think it
>> > checks for the display attribute. it will also break 3rd party
>> javascript
>> > libs that dont check for display attrs.
>> >
>> > i see how this makes life easier, but it also has a potential to be
>> evil.
>> > that is why i closed the issue as wont-fix and told vincent to
>> bring the
>> > discussion here on the list before we go any further.
>> >
>> > -igor
>> >
>> >
>> > On 3/19/07, Matej Knopp <[EMAIL PROTECTED]> wrote:
>> > >
>> > > I don't like it. I don't think deprecating set/isVisible is the
>> way to
>> > > go. Plus there are other reasons: As now we use one flag for
visible
>> > > status. With your approach you'd require an enum, int, byte or
>> > > something similiar, that takes more space then just one bit in
>> flags.
>> > >
>> > > I think we can just make a getter, that by default returns the
value
>> > > from application settings. And you can override that for your
>> > > component, if you want something different that what's set in
>> > > application settings.
>> > >
>> > > -Matej
>> > >
>> > > On 3/19/07, Vincent Demay <[EMAIL PROTECTED]> wrote:
>> > > > Frédéric Bertin a écrit :
>> > > > > Martijn Dashorst wrote:
>> > > > >>> it seems taht this kind of construction is used to make
>> workaround
>> > > of
>> > > > >>> the bug. Is'n it?
>> > > > >>
>> > > > >> First, what bug? I don't know that this is a bug? I thought we
>> are
>> > > > >> discussing a feature here. Secondly, this is not a workaround,
>> but
>> > > > >> creating client side code based on a API contract:
>> setVisible(false)
>> > > > >> removes the component markup completely, including its tags,
>> from
>> the
>> > > > >> final markup.
>> > > > > ok, then there's a need for 2 different mechanisms.
>> > > > > One to switch a component visibility. This one should be
>> *reversible*
>> > > > > (in ajax or not), and then it should always output a tag with
an
>> id
>> > > > > attribute (actually, can be done only when setOutputMarkupId is
>> set to
>> > > > > true).
>> > > > >
>> > > > > Another to render or not a component in the output markup. This
>> one
>> > > > > should be documented as *not reversible*. This is the current
>> > > > > setVisible implementation.
>> > > > >
>> > > > > wdyt?
>> > > >
>> > > > +1
>> > > > What about keeping current behavior on setVisible (deprecated)
and
>> > > > add a method setVisibility :
>> > > > - none : render nothing
>> > > > - visible : render all
>> > > > - invisible : render only container tag with
>> style:display-none
>> > > >
>> > > > Add in documentation
>> > > > none: can not become visible in ajax
>> > > > in visible: can
>> > > >
>> > > > I think it will match to all use case, no ?
>> > > > >
>> > > > >
>> > > > >
>> > > > >>
>> > > > >> It is based on the assumption that some element is *NOT*
>> present
>> at
>> > > > >> all. Your change will invert that behavior, and in such a way
>> that it
>> > > > >> is only detectable by debugging your javascript. Not
>> something I
>> > > > >> enjoy, nor 95% of the development community.
>> > > > >>
>> > > > >> You must understand that this is a major api break, not
>> something
>> > > > >> minor. This is not detectable by a compiler. You *will* break
>> > > existing
>> > > > >> scripts, pages and applications in a non-obvious way. Silent
>> failures
>> > > > >> are something we try to avoid at all cost.
>> > > > >>
>> > > > >> Martijn
>> > > > >>
>> > > > >> On 3/19/07, Vincent Demay <[EMAIL PROTECTED]>
>> wrote:
>> > > > >>> Martijn Dashorst a écrit :
>> > > > >>> > Currently everybody assumes (correctly) that the element is
>> > > > >>> completely
>> > > > >>> > removed (Ajax and non-Ajax)
>> > > > >>> Yes of course, but it is the same for all workarounds ;)
>> > > > >>> When to change servlet to filter, users have to change
>> their web
>> > > xml.
>> > > > >>> Each time you change something users have to adapt their code
>> > > > >>> > i.e. not present in the final markup.
>> > > > >>> > This means that scripts that iterate through the dom, or
>> check
>> for
>> > > > >>> the
>> > > > >>> > document.getElementById() == null will fail if we implement
>> this.
>> > > > >>> >
>> > > > >>> it seems taht this kind of construction is used to make
>> workaround
>> > > of
>> > > > >>> the bug. Is'n it?
>> > > > >>> > I *strongly* discourage changing this behavior.
>> > > > >>> >
>> > > > >>> > Martijn
>> > > > >>> >
>> > > > >>> > On 3/19/07, Matej Knopp <[EMAIL PROTECTED]> wrote:
>> > > > >>> >> Will it? This seems to be actually quite a smart
>> workaround.
>> How
>> > > > >>> >> exactly will this break existing clients? Only thing i'm
>> > > concerned
>> > > > >>> >> about is the validity of output markup. but imho when we
>> preserve
>> > > > >>> the
>> > > > >>> >> original tag names, e.g. td will render as td, it should
be
>> all
>> > > > >>> right.
>> > > > >>> >>
>> > > > >>> >> -Matej
>> > > > >>> >>
>> > > > >>> >> On 3/19/07, Martijn Dashorst <[EMAIL PROTECTED]>
>> wrote:
>> > > > >>> >> > So you mean:
>> > > > >>> >> >
>> > > > >>> >> > Label l = Label("foo", "hello");
>> > > > >>> >> > renders:
>> > > > >>> >> > <span wicket:id="foo">hello</span>
>> > > > >>> >> >
>> > > > >>> >> > ... some ajax stuff, or a normal page render:
>> > > > >>> >> >
>> > > > >>> >> > l.setVisible(false);
>> > > > >>> >> > renders:
>> > > > >>> >> > <span wicket:id="foo" style="display:none"></span>
>> > > > >>> >> > ?
>> > > > >>> >> >
>> > > > >>> >> > This can and will break existing clients in a very nasty
>> > > manner,
>> > > > >>> >> > because the markup id is still present in the final
>> markup.
>> > > > >>> >> >
>> > > > >>> >> > Martijn
>> > > > >>> >> >
>> > > > >>> >> > On 3/19/07, Vincent Demay
>> <[EMAIL PROTECTED]>
>> > > wrote:
>> > > > >>> >> > > Johan Compagner a écrit :
>> > > > >>> >> > > >> > Also always just rendering the component but
>> use the
>> > > style
>> > > > >>> >> to make in
>> > > > >>> >> > > >> > invisible
>> > > > >>> >> > > >> > could be a security problem. So that can't be the
>> > > default.
>> > > > >>> >> > > >>
>> > > > >>> >> > > >> What do you mean by security problem?
>> > > > >>> >> > > >
>> > > > >>> >> > > >
>> > > > >>> >> > > >
>> > > > >>> >> > > > If the the component that is set to none visible is
>> none
>> > > > >>> visible
>> > > > >>> >> > > > because of
>> > > > >>> >> > > > security
>> > > > >>> >> > > > So it has data that never should be send to the
>> browser
>> > > > >>> because
>> > > > >>> >> the
>> > > > >>> >> > > > user is
>> > > > >>> >> > > > not allowed
>> > > > >>> >> > > > to see it.
>> > > > >>> >> > >
>> > > > >>> >> > > But data is never send to the user because a none
>> visible
>> > > > >>> >> component will
>> > > > >>> >> > > be render as an empty tag, so without data
>> > > > >>> >> > >
>> > > > >>> >> > >
>> > > > >>> >> >
>> > > > >>> >> >
>> > > > >>> >> > --
>> > > > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
>> > > > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
>> > > > >>> >> > Wicket 1.2.5 will keep your server alive. Download
Wicket
>> now!
>> > > > >>> >> > http://wicketframework.org
>> > > > >>> >> >
>> > > > >>> >>
>> > > > >>> >
>> > > > >>> >
>> > > > >>>
>> > > > >>>
>> > > > >>
>> > > > >>
>> > > > >
>> > > > >
>> > > >
>> > > >
>> > >
>> >
>>
>
