neo09 / gwt-platform

Automatically exported from code.google.com/p/gwt-platform
0 stars 0 forks source link

Attaching a PresenterWidget to a new parent messes up the lifecycle #140

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
See this comment in PresenterWidgetImpl line 152 :
// Do nothing if the content is already added

I think that if it's already added, then it should at least show it.

Use case : On my view

    @UiHandler("focusPanel")
    void onFocusPanelClicked(ClickEvent event) {
        if (!showMenu) {
            presenter.onShowGadgetLibrary();
        }
    }

On my presenter

    @Override
    public void onShowGadgetLibrary() {
    addPopupContent(gadgetLibraryPopupPresenter, true);
    }

That way I wont have to do a first check.

I'll fixe this depending on feedback.

Original issue reported on code.google.com by goudreau...@gmail.com on 18 Jul 2010 at 12:48

GoogleCodeExporter commented 9 years ago
I'm not sure I get this. If it's "added" it should be visible. PopupView are 
removed from their parent presenter's list as soon as you hide() them.

Original comment by philippe.beaudoin on 18 Jul 2010 at 12:54

GoogleCodeExporter commented 9 years ago
Hummm, well, hide is called.

Then it's not removed from popupChildren list properly since I have a hit even 
after hide()

Original comment by goudreau...@gmail.com on 18 Jul 2010 at 1:00

GoogleCodeExporter commented 9 years ago
but... w8, it may be because I use the same popup for more than one widget :D

Original comment by goudreau...@gmail.com on 18 Jul 2010 at 1:02

GoogleCodeExporter commented 9 years ago
After discussion, this is a more important bug:

If a singleton PresenterWidget (and maybe even a Presenter) switches its parent 
presenter, it may be present in two trees of activeChildren (or popupChildren). 
We should work on a unit test that exposes this bug.

Original comment by philippe.beaudoin on 18 Jul 2010 at 1:16

GoogleCodeExporter commented 9 years ago
Meanwhile, a quick fix would be to make sure your view call:
setAutoHideOnNavigationEventEnabled(true)

in its constructor.

Original comment by philippe.beaudoin on 18 Jul 2010 at 1:29

GoogleCodeExporter commented 9 years ago
This has been fixed. It's simple but relatively involved, code reviews would be 
much appreciated:
http://codereview.appspot.com/1731052/show

Original comment by philippe.beaudoin on 18 Jul 2010 at 5:26

GoogleCodeExporter commented 9 years ago

Original comment by philippe.beaudoin on 18 Jul 2010 at 5:34

GoogleCodeExporter commented 9 years ago

Original comment by philippe.beaudoin on 18 Jul 2010 at 5:34