Removing NonBlockingGenericDialogs from the Window menu

[dscho]

Dear Wayne,

I just noticed your fix:

https://github.com/imagej/ImageJA/commit/fb84ee03bedc20c141716acd34ee6ad8c8e32c6d#diff-f393bd19cc4b42d8d8549c384ce229e3L21

However, removing the window only when a button was clicked or a key was
pressed is very likely to miss the case where the dialog was closed
programmatically (e.g. when the dialog was only valid through the life
time of a long-running process, and closed when that process was done).

While looking at the larger context, I also could not help but notice that
the dialog is added to the Window menu long before the dialog is shown.
And indeed, it is added to the Window menu even if the dialog is not shown
at all (for which there are quite a few use cases -- I trust your
imagination to come up with even more than I could think of).

A better practice would be to implement something like this (sorry, I have
no time to test this, besides, you made clear that you like to implement
such changes yourself and ignore pull requests, therefore I will not
burden you with one):

        @Override
        public void setVisible(boolean visible) {
                if (!visible) {
                        WindowManager.removeWindow(this);
                }
                super.setVisible(visible);
                if (visible) {
                        WindowManager.addWindow(this);
                }
        }

Regards,
Johannes

--
ImageJ mailing list: http://imagej.nih.gov/ij/list.html

[dscho]

Dear Wayne,

On Wed, 5 Mar 2014, Rasband, Wayne (NIH/NIMH) [E] wrote:

> On Mar 3, 2014, at 5:11 PM, Johannes Schindelin wrote:
>
> > While looking at the larger context, I also could not help but notice
> > that the dialog is added to the Window menu long before the dialog is
> > shown.  And indeed, it is added to the Window menu even if the dialog
> > is not shown at all (for which there are quite a few use cases -- I
> > trust your imagination to come up with even more than I could think
> > of).
>
> I moved "WindowManager.addWindow(this)" from the constructor to the
> showDialog() method, which should help.

Thank you.

> > A better practice would be to implement something like this (sorry, I
> > have no time to test this, besides, you made clear that you like to
> > implement such changes yourself and ignore pull requests, therefore I
> > will not burden you with one):
> >
> > @Override
> > public void setVisible(boolean visible) {
> > if (!visible) {
> > WindowManager.removeWindow(this);
> > }
> > super.setVisible(visible);
> > if (visible) {
> > WindowManager.addWindow(this);
> > }
> > }
>
> I could not get this to work. The setVisible() method is not called when
> the user closes the dialog.

My point was not to have this particular method removing the window from
the WindowManager. My point was to have a method that is always called
when the window closes do that. I guess dispose() would be called in that
case.

Ciao,
Johannes

--
ImageJ mailing list: http://imagej.nih.gov/ij/list.html