Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scala.swing.Window.contents_=() should not call pack() #132

Open
scabug opened this issue Apr 21, 2010 · 4 comments
Open

scala.swing.Window.contents_=() should not call pack() #132

scabug opened this issue Apr 21, 2010 · 4 comments
Assignees

Comments

@scabug
Copy link

scabug commented Apr 21, 2010

in trunk as of april 22 2010:

scala.swing.Window:

37  override def contents_=(c: Component) {
38	    super.contents_=(c)
39	    peer.pack() // pack also validates, which is generally required after an add
40	  }

As can be seen, 'pack()' is called when changing the content of a Window. This is generally NOT what you want. Pack() has the function to resize a window to fit its contents. It's an optional feature. It is normally NOT desired when the programmer sets the size explicitly (using size = ...). It is in general NOT desired when the window has a size set before.

This can cause the window of the app to be suddenly resized when it sets e.g. a new mainpanel in it. E.g. a bank-app; Window resizes when the initial overview screen is installed; resizes again (!) when this is replaced by some detail screen.

To make things worse, imagine the end-user to resize the window to it's liking, which will be undone when the content of the window changes! (caused by this 'pack()')! That would def. be regarded as a bug.

It MIGHT be a nice initial, one-time, fallback auto-behaviour in the case when the window doesn't have a size set yet, but is requested to be displayed. (The pack() call should be moved to that function then and some condition state-checking should be added to the class).[[BR]]
BUT EVEN THEN, I am not sure that there are no valid cases for wanting to display a window with a 'zero' size (if that's the default for a window).

@scabug
Copy link
Author

scabug commented Apr 21, 2010

Imported From: https://issues.scala-lang.org/browse/SI-3335?orig=1
Reporter: me (scalll)

@scabug
Copy link
Author

scabug commented Apr 26, 2010

@ingoem said:
The vast majority of use cases for contents_= is to set the initial contents of a window and there you want a pack or at least a validate. Changing this now would break quite some code. Moreover, I would consider changing the entire content of a window in the middle of an app run bad style anyways. If you really want to do that, you should be able to do so with one level of indirection: add a panel and change it's contents instead of the window's contents.

@scabug
Copy link
Author

scabug commented Apr 26, 2010

me (scalll) said:
Hi Ingo,
you write:

The vast majority of use cases for contents_= is to set the initial contents of a window and there you want a pack or at least a validate. Changing this now would break quite some code. 

You could replace the pack() call to a revalidate() call (and perhaps a repaint(), if I remember well).

Moreover, I would consider changing the entire content of a window in the middle of an app run bad style anyways.

Could you explain that?
To me it seems the most clean and straightforward way. In full accordance with the swing specs.

If you really want to do that, you should be able to do so with one level of indirection: add a panel and change it's contents instead of the window's contents.
{code}
and not needing borderplate work-arounds like this.
In normal Swing I can just cleanly replace the contents. (Which is supported, check the API docs).

Ofcourse I understand that you want your DSL as simple as possible, and like to get rid of an explicit pack(), which people tend to forget.
That's why I propose you could look to use pack() (and/or validate()) conditionally, not in the contents update method, but at a 'just before displaying' point.
In the other case programmers set the size of a window explicitly (by using size = ...).

I appreciate all the work that has gone in the scala Swing API. It makes swing programming a lot easier and cleaner. But this peticular pack() call hurts more than that it's helpful.

@scabug
Copy link
Author

scabug commented Apr 26, 2010

me (scalll) said:
(Oops, messed up formatting, again..)

Hi Ingo, you write:

The vast majority of use cases for contents_= is to set the initial contents of a window and there you want a pack or at least a validate. Changing this now would break quite some code. 

You could replace the pack() call to a revalidate() call (and perhaps a repaint(), if I remember well).

Moreover, I would consider changing the entire content of a window in the middle of an app run bad style anyways. 

Could you explain that?
To me it seems the most clean and straightforward way. In full accordance with the swing specs.

If you really want to do that, you should be able to do so with one level of indirection: add a panel and change it's contents instead of the window's contents.

and not needing borderplate work-arounds like this.
In normal Swing I can just cleanly replace the contents. (Which is supported, check the API docs).

Ofcourse I understand that you want your DSL as simple as possible, and like to get rid of an explicit pack(), which people tend to forget.
That's why I propose you could look to use pack() (and/or validate()) conditionally, not in the contents update method, but at a 'just before displaying' point.
In the other case programmers set the size of a window explicitly (by using size = ...).

I appreciate all the work that has gone in the scala Swing API. It makes swing programming a lot easier and cleaner. But this peticular pack() call hurts more than that it's helpful.

@scabug scabug closed this as completed Jul 17, 2015
@SethTisue SethTisue transferred this issue from scala/bug Nov 19, 2020
@SethTisue SethTisue reopened this Nov 19, 2020
@scala scala deleted a comment from scabug Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants