Third-party widgets outside Widget UI package

[Folke]

Hi

Short question about third-party widgets and the methods
Widget.setParent(), .onAttach(), and .onDetach().

Because these methods are package-private and protected respectively,
can someone confirm that widgets living outside
com.google.gwt.user.client.ui do not have the same freedom as those
that do?

I'm asking because of this thread:
http://groups.google.com/group/Google-Web-Toolkit/browse_thread/thread/78c45552015ea220

The question is if someone can implement something complex like Tree
but outside the UI package, or more general, a widget that contains
other Widgets but does not extend Panel or Composite. (Rationale:
Panel exposes do many methods and there's no initial Widget that can
be fed to Composite.)

package com.example.not.in.gwt.ui;

public class MyWidget extends Widget {
private Widget childWidget;

protected void doAttachChildren() {
childWidget.onAttach(); // ERROR, onAttach() is protected
}

protected void doDetachChildren() {
childWidget.onDetach(); // ERROR, onDetach() is protected
}

public void setChildWidget(Widget w) {
w.removeFromParent();
// adoption process
childWidget.setParent(this); // ERROR, setParent() is package-
private
}
}

Thanks!

Folke

[Emily Crutcher]

Actually, Tree's new version (FastTree in incubator), has been written to extend Panel. The idea is that the attachment code is very tricky and easy to introduce memory leaks to, so we try to provide the most resilient API's we can, in this case Panel and Composite.

The one problem we have with Panel is that is has a generic "add" method. This is somewhat frustratingly, but unfortunately is a feature shared by HasWidgets, so if you want your container to play nicely with any of the other GWT widgets, it is unavoidable.

I'd be interested if you have a specific use case where extending from Panel or Composite is inappropriate, as if we need a third type of container, we'd love to know about it!

--
"There are only 10 types of people in the world: Those who understand binary, and those who don't"

[Folke]

Thanks for the explanation, Emily!

I'm really happy with how things are. I was just curious because I ran
into the same "problem" before I saw the beauty in the simplicity and
safety of Panel and Composite. I haven't done many panels but I use
Composite a lot.

What I meant was that the strictness certainly divides developers into
two groups: the ones that enjoy the simple and safe way and the others
that feel patronized by it. See the referred thread if you like to see
both types. One guy, Diego, wants to build a widget with child widgets
but without the need to implement add(), remove(), Iterator.remove(),
etc., and he also doesn't have a child widget for
Composite.initWidget(). (Of course, he could just use FlowPanel and
add wrap his widget in a <div>.)

That said, I'm not sure if it's necessary to introduce a third
container class that opens up the parent/child widget chaining for
third party widgets. (Maybe yes, but with a lot of warnings.) That's
up to you. I'm perfectly fine for now.

Thanks again!

Regards,
Folke

On Aug 18, 5:59 pm, "Emily Crutcher" <e...@google.com> wrote:
> Actually, Tree's new version (FastTree in incubator), has been written to
> extend Panel. The idea is that the attachment code is very tricky and easy
> to introduce memory leaks to, so we try to provide the most resilient API's
> we can, in this case Panel and Composite.
>
> The one problem we have with Panel is that is has a generic "add" method.
> This is somewhat frustratingly, but unfortunately is a feature shared by
> HasWidgets, so if you want your container to play nicely with any of the
> other GWT widgets, it is unavoidable.
>
> I'd be interested if you have a specific use case where extending from Panel
> or Composite is inappropriate, as if we need a third type of container, we'd
> love to know about it!
>
>
>
> On Sun, Aug 17, 2008 at 8:19 PM, Folke <mess...@gmail.com> wrote:
>
> > Hi
>
> > Short question about third-party widgets and the methods
> > Widget.setParent(), .onAttach(), and .onDetach().
>
> > Because these methods are package-private and protected respectively,
> > can someone confirm that widgets living outside
> > com.google.gwt.user.client.ui do not have the same freedom as those
> > that do?
>
> > I'm asking because of this thread:
>

> >http://groups.google.com/group/Google-Web-Toolkit/browse_thread/threa...

[Joel Webber]

We'd certainly consider extending this model to deal with new use-cases. But from reading this thread, I'm not sure that any truly new cases are described (the fact that Tree uses protected APIs is an oversight we need to fix, but Emily's FastTree example shows the exact same thing "done right").

The problem with implementing a "widget that has child widgets, but doesn't implement HasWidgets" is that it is virtually *guaranteed* to leak memory. Lots of it (typically the whole bloody UI structure, because almost everything is reachable from the leaked widget). The code in RootPanel.detachWidgets() (and a few other places) has to be able to assume that the entire widget hierarchy will be properly detached and cleaned up. If any 'pseudo-container' breaks this chain, it will definitely cause a leak (at least on IE, but not solely).

[Folke]

Hi Joel

I don't think Panel is the problem. Diego is more interested in a way
to use a widget that manages its child widgets. Nothing can be added
or removed from the outside. Yes, a perfect case for Composite+Panel.
I understand that and so does he. But he doesn't want use an inner
Panel for whatever reason. Maybe he's just purist, maybe he does have
a valid case. I don't know. His point is that he already has a widget
and he wants to make it the parent widget of his child widgets and not
add an intermediate Panel that adds its own element(s).

(I now write from my perspective instead of his, but like i said above
I'm happy with how things are. I just want to make sure that you
understand that this is more about Composite than Panel.)

Let's say I want to write a dynamic widget with lots of DOM
operations, and this widget contains several text boxes. For these I
wish to use TextBox and ChangeListener instead of reimplementing this.
Now purely hypthetically, I'm a smart developer and I know all the
bloody intrinsics of child widget adoption and attachment, really hard
stuff, but it's not possible for me to correctly embed the TextBoxes
inside my DOM, because I can't use the necessary methods like
setParent().

Well, I don't know of any _real_ use-cases but you could pull
UIObject, Widget, Panel and Composite out of the UI package (or put
the widgets in a widget package) and see if Tree is the only class
with errors. Label.wrap() comes to my mind where onAttach() is called
on the new Label. If I were to have such an idea I would have to ask
the GWT team for this functionality before I could use it.

Maybe, if you open up the API there will be more reasons to open up
the API.

Best regards,
Folke

[Joel Webber]

I do understand where everyone's coming from here, but I'm still a little unclear on what problem is solved by not using a Panel/Composite pair. I checked callers of setParent(), and Tree is the only widget that makes that mistake, and can easily be fixed.

And I'm not wholly *against* making it possible for widgets in outside packages to do the same thing that Panel does, but it would have to be done so in a way that makes it very difficult to screw up. Right now, if setParent() were public, then I strongly suspect people will start calling it for various weird reasons, which will appear to work much of the time. Then they will start getting bizarre errors (and possibly leaks) because they've put the widget hierarchy in an inconsistent state that's difficult to detect.

[Folke]

Hi Joel

Here's my new Container class with an example usage scenario at the
bottom. Container looks like a Panel with the exception that it
doesn't implement HasWidgets. In fact, I think of Container as a
potential super-class of Panel. It's slim and introduces only two new
methods that are safe to call. I think any use-case is valid where you
need to extend Panel but do not want to implement HasWidgets because
all child widgets are managed within the class and are not added or
removed outside the class. Of course, this can also be done with
Composite+HTMLPanel and the result is even small but it's not as
flexible.

http://docs.google.com/Doc?id=dgf5d73j_17c99djxgc

I understand that you do not want other developers to get shot in the
foot and then probably take the heat for it (or that the issue tracker
gets filled up). That's ok. I'm not pursuing this anymore but I hope
you could say a few words about Container and I'll leave it at that.

Thanks in advance!

Regards,
Folke

[Joel Webber]

Your Container class doesn't seem unreasonable at all to me. As you say, it seems like a middle-ground between Widget and Panel, but it still enforces consistency between a widget's parent and its attached state, which is good.

I do have a few concerns about it:

- A Container would be opaque to child enumeration, which might be a problem for things like drag & drop (though you could arguably consider this a feature, if that's what you're trying to achieve).

- The Container subclass would need to call Widget.removeFromParent() (just like most Panels) to avoid getting the widget hierarchy in an inconsistent state (e.g. if you tried to add a Composite's widget to the Container, removeFromParent() would correctly fail).

- It introduces a new situation where a Widget cannot be removed from its parent. Currently this can only happen for Widgets passed to Composite.initWidget() (see Widget.removeFromParent()). Not the end of the world, but it could be potentially confusing when you try to add a container's child to another panel.

Still an interesting idea, though.

Cheers,

joel.

[Folke]

Thanks for your reply. I'm glad Container meets your basic
requirements. I'm planning on introducing Container at work and see
how well it fits in. Maybe it isn't worth the trouble.
(I think of one particular class that extends Panel but makes all four
HasWidgets methods throw UnsupportedOperationExceptions. All child
widgets are managed internally and aren't exposed.)

I addressed your concerns below.

On Aug 22, 4:51 pm, "Joel Webber" <j...@google.com> wrote:
> Your Container class doesn't seem unreasonable at all to me. As you say, it
> seems like a middle-ground between Widget and Panel, but it still enforces
> consistency between a widget's parent and its attached state, which is good.
> I do have a few concerns about it:
> - A Container would be opaque to child enumeration, which might be a problem
> for things like drag & drop (though you could arguably consider this a
> feature, if that's what you're trying to achieve).

I haven't tried any D&D implementations yet but I would think that
they should treat any Widget atomically. In fact, they have to if
they're not inside the GWT's UI package. And even then no
implementation can know about all child widgets because
HasWidgets.iterator() might not return all widgets, i.e., if you have
some sort of caption that is a Label or an HTML widget. This widget
does not belong to the collection of widgets that are added by
HasWidgets.add().

What I cannot answer is how to make a Container act as a drop target
or any of its children, or how to make the children be draggable
outside the container. That's very implementation specific. I'll take
a look at the gwt-dnd project. But adding and removing other widgets
directly to a Container should not be possible. (See below.) That's
what Panels are for.

> - The Container subclass would need to call Widget.removeFromParent() (just
> like most Panels) to avoid getting the widget hierarchy in an inconsistent
> state (e.g. if you tried to add a Composite's widget to the Container,
> removeFromParent() would correctly fail).

Container subclasses create and dispose of their child widgets. No
need for removeFromParent(). Besides, removeFromParent() will throw an
IllegalStateException, as you implied below. Containers usually don't
implement HasWidgets. That's the whole point. If you expose a child
widget, like I did in my example (getNameBox), then other classes
cannot rip it out by calling removeFromParent(). That wasn't planned
but it's a nice side effect.

On the other hand, Container could provide a "protected"
remove(Widget) method (which could be made "public" by Panel) and
Widget.removeFromParent() could be changed to check the parent against
HasWidgets and Container.

Container:
protected boolean remove(Widget child) {
throw new UnsupportedOperationException();
}


Panel (extends Container):
public abstract boolean remove(Widget child);


Widget:
public void removeFromParent() {
if (parent instanceof Container) {
((Container) parent).remove(this);
} else if (parent instanceof HasWidgets) {
((HasWidgets) parent).remove(this);
} else if (parent != null) {
throw new IllegalStateException(
"This widget's parent does not implement HasWidgets");
}
}

Now people can override Container.remove() to allow safe "extraction"
of child widgets.

Container now has a protected remove() method:
http://docs.google.com/View?docId=dgf5d73j_17c99djxgc

> - It introduces a new situation where a Widget cannot be removed from its
> parent. Currently this can only happen for Widgets passed to
> Composite.initWidget() (see Widget.removeFromParent()). Not the end of the
> world, but it could be potentially confusing when you try to add a
> container's child to another panel.

Well, child widgets should not be removable because no one is supposed
to know about them.
There's only Container! ;)

Have a nice weekend!

Folke