RR: Attach/Detach behavior of panels that show and hide child Widgets and the onLoad method

443 views
Skip to first unread message

Sandy McArthur

unread,
Apr 6, 2007, 1:04:05 AM4/6/07
to Google Web Toolkit Contributors
I've asked for part of this twice now so I thought maybe I could get
my desired behavior for Panels to be the "official" one, at least for
new Panels. (I'm really being somewhat selfish because if I'm the only
one who thinks this way then I plan to make local copies and edit some
of GWT provided Panels to get this behavior.)

What:
I'd appreciate it if Panels that show/hide their child content Widgets
(DeckPanel, StackPanel and a number of new for GWT 1.4 panels) would
detach the child Widgets while they are not shown. Right now they tend
to simply call setVisible(false) which sets the style display to none
to hide the Widget.

Also, I'd like to suggest that the Widget.onLoad() method be redefined
so it is only called once when a Widget is first attached to the
browser's document.

Why:
I can think of a couple of reasons to attach/detach Widgets instead of
hiding them:

1. Simply hiding a Widget with setVisible(false) still leaves the
Widget's elements as part of the browser's document. Presumably a
bigger document DOM is going to take longer to re-render and traverse.

2. Attaching and detaching Widgets as opposed to showing or hiding
them provides an opportunity for lazy initialization or optimizing
performance.

2.1. I've been writing my custom widgets so they don't fully render
themselves until they are attached. While this does make the actual
onAttach() call slower it means I'm potentially avoiding work that
isn't ever needed.

2.2. I've started writing my widgets so that when they are detached
they don't update themselves. The best example of this is the table I
have in my GWT-Stuff project. It automatically updates the table rows
based on changes to an observable List. Rendering 100 table row isn't
a trivial operation and one thing I like to do is provide multiple
views of the same data so I have a summary table view and a detail
table view backed by the same List instance and both stay in sync.
When the table is detached it does the minimum bookkeeping needed to
track changes to the List. When it is attached it scans it's internal
structures and populates the new rows with the data. This way elements
that are added and removed to the observed List while the table is
detached never get rendered.

As for changing onLoad to only be called once at the first attach:
1. Right now if a custom widget wants to do one time initialization
they either need to manually keep track of if they have been
initialized to prevent double initialization. Or do all the
initialization in the constructor.

1.1. Allowing the programmer to not have to check for double
initialization would make the compiled JavaScript smaller because of
reduced code duplication. This is a similar concern to bloat from null
object checks scattered all over the place.

1.2. Doing all of the initialization in the constructor slows the
application's start up time because you cannot easily defer unneeded
initialization code.

Disadvantage:
1. The most persuasive reason not to do this is onDetach/onAttach will
be slower than calling setVisible(boolean) because all the children of
the parent have to be attached or detached too while calls to the
parent's setVisible doesn't cascade calls to it's children widgets
like that. In my experience so far the slightly slower
onAttach/onDetach method for show/hide isn't noticeable if the
onAttach/onDetach methods aren't doing any serious work. In the case
of my table, if the entire list of elements has changed and the whole
table needs to be re-rendered then yes I notice a slight delay in the
calls to onAttach(). I find this acceptable because I know my app is
doing a lot of work and it was faster other times when it deferred
work until now.

2. FormPanel has a internal iframe that it uses. A FormPanel that is
submitted and then detached while the user waits for the submit to
finish could lead to a unexpected behavior. I'm not sure what would
happen but it would need to be tested to make sure nothing breaks.

3. This would be a change in behavior in the onLoad() method but I
think it was the original intended behavior. At least it's the
intuitive behavior. A JavaScript programmer expects the browser's
document onload event to only be fired once so it seems a little
strange that the Widgets.onLoad() method can be called twice. Plus,
given the current behavior of DeckPanel and StackPanel onAttach() is
only called once and thus onLoad() is only called once unless the
programmer removes them programmatic.

Alternatives:
* Override setVisible to track the attached/detached state and make
the needed updates. This doesn't support multiple levels of lazy
initialization because a child of a hidden parent won't know to
suspect updates because it has no way to detect when it's hidden as a
side effect of it's parent being hidden. Also, setVisible is a
UIObject method and not specifically a Widget method.

--
Sandy McArthur

"He who dares not offend cannot be honest."
- Thomas Paine

Fred Sauer

unread,
Apr 6, 2007, 1:41:35 AM4/6/07
to Google-Web-Tool...@googlegroups.com
On 4/5/07, Sandy McArthur <sand...@gmail.com> wrote:
Also, I'd like to suggest that the Widget.onLoad() method be redefined
so it is only called once when a Widget is first attached to the
browser's document.

Sort of +1 here. I'd like it if onLoad behaved as it did today (firing each time a widget gets (re)attached), but add a new method onInitialLoad() which just fires once. Compiler should optimize out everyone who does not override the otherwise empty method.

My use case for having both onLoad/onInitialLoad is for when I drag widgets around (in my gwt-dnd project) and end up (for every drag/drop operation) detaching and reattaching widgets over and over again.

There are three types of initialization which I end up doing in my widgets:
  1. Stuff in the constructor - you know what goes there
  2. On initial load - things which can only be calculated once we are attached to the DOM, e.g. depend on retrieving pixel size, CSS borders, etc. Also lazy initialization type stuff which Sandy talks about
  3. On every load - hook up event listeners (handler); refresh state; etc.
So, two methods for 2 + 3 please: onLoad, onInitialLoad.

Thanks--
Fred Sauer
fr...@allen-sauer.com

Mathias Bogaert

unread,
Apr 9, 2007, 4:40:54 AM4/9/07
to Google Web Toolkit Contributors
I'd be very happy to see this as well.

On Apr 6, 7:41 am, "Fred Sauer" <f...@allen-sauer.com> wrote:


> On 4/5/07, Sandy McArthur <sandy...@gmail.com> wrote:
>
>
>
> > Also, I'd like to suggest that the Widget.onLoad() method be redefined
> > so it is only called once when a Widget is first attached to the
> > browser's document.
>
> Sort of +1 here. I'd like it if onLoad behaved as it did today (firing each
> time a widget gets (re)attached), but add a new method onInitialLoad() which
> just fires once. Compiler should optimize out everyone who does not override
> the otherwise empty method.
>
> My use case for having both onLoad/onInitialLoad is for when I drag widgets
> around (in my gwt-dnd project) and end up (for every drag/drop operation)
> detaching and reattaching widgets over and over again.
>
> There are three types of initialization which I end up doing in my widgets:
>

> 1. Stuff in the constructor - you know what goes there
> 2. On initial load - things which can only be calculated once we are


> attached to the DOM, e.g. depend on retrieving pixel size, CSS
> borders, etc. Also lazy initialization type stuff which Sandy talks about

> 3. On every load - hook up event listeners (handler); refresh state;


> etc.
>
> So, two methods for 2 + 3 please: onLoad, onInitialLoad.
>
> Thanks--
> Fred Sauer

> f...@allen-sauer.com

Fred Sauer

unread,
Apr 9, 2007, 11:39:10 AM4/9/07
to Google-Web-Tool...@googlegroups.com
I've created http://code.google.com/p/google-web-toolkit/issues/detail?id=896 for the onInitialLoad() method.

You can star that issue to indicate your vote/interest.
Reply all
Reply to author
Forward
0 new messages