Created attachment 22300 [details] [review]
This patch should fix the hpaned parameter (horizontal size).

Created attachment 22350 [details] [review]
This patch uses gtk_widget_set_size_request calls to set the minimum height of the lists to 180 pixels.

The minimum and default size with the second patch is now similar to
the size of GtkFileSelection. I think that should be an acceptable
solution as long as there is no better way to solve it.
If required we could add a gtk_window_set_default_size call in
GtkFileChooserDialog to make the default size a bit larger than the
minimum size.

hmm, seems still sort of hard-wired to me.
Can we base the minimum size (reliably) on the current font size,
etc., in accordance with the theme?  So that even if a large-size
theme (thick focus lines, etc.) is chosen, with a large font, the list
has some minimum number of lines visible (say, three? )?  Just
wondering how this will play with both small screens and really big
fonts and focus lines (i.e. low-vision themes).

It is possible to let the size request depend on the font size:

  test_renderer = g_object_new (GTK_TYPE_CELL_RENDERER_TEXT,
                                "text", "TEST LINE",
                                NULL);
  gtk_cell_renderer_get_size (test_renderer, impl->list, NULL,
                              NULL, NULL, NULL, &test_height);
  g_object_unref (test_renderer);
  gtk_widget_set_size_request (impl->list, -1, test_height * 5);

This seems to be a good approximation (although a font size > 40 shows
only 4.75 lines) but I don't know if such a hack is really a good
solution.

A "proper" solution would depend on bug 101859, so we will probably
have to get by with the hack for a few years.

I applied the pack1/pack2 patch (22300).  Thanks, Jan Arne.

See bug #129318 for a proposed "visible-rows" API for GtkTreeView.

*** Bug 132669 has been marked as a duplicate of this bug. ***

>  test_renderer = g_object_new (GTK_TYPE_CELL_RENDERER_TEXT,
>                                "text", "TEST LINE",
>                                NULL);

Code that does this is not going to work very well in a multiscreen
setup.  Unless you install it also in the screen-change signal handler.
Basically you don't know what screen to do the sizing on at this stage.

Fixed on CVS.

2004-02-13  Federico Mena Quintero  <federico@ximian.com>

	Fix #129020.

	* gtk/gtkfilechooserdialog.c
	(set_default_size): New function, sets a reasonable default size
	for the window.
	(gtk_file_chooser_dialog_realize): New function, call
	set_default_size().
	(gtk_file_chooser_dialog_style_set): Likewise.
	(gtk_file_chooser_dialog_screen_changed): Likewise.

	* tests/testfilechooser.c (main): Don't set a default size for the
	dialog.

*** Bug 132479 has been marked as a duplicate of this bug. ***

Created attachment 24815 [details] [review]
Additional patch...

The current implementation essentially ignores the size the widget wants
to have, ie., MAX prevails.  That's wrong.  For example, add a 100 pixel
wide preview and the default size does not change.

With my extra patch (which assumes the panes to be empty when called
-- is that right?) will _add_ the width of a smaller number of characters.

Lowering priority to normal since the current size isn't absurd.

Removing the TARGET keyword too; it's no longer a showstopper, though
I suppose fixing the problem Morten has described would be nice.

What about simply realizing the dialog before setting the size? Ie.,
something like the patch I'll attach.

Created attachment 25397 [details] [review]
patch

The clamp-to-screen part of the patch is good.  I was going to write
something similar but never got the time.  I don't think that doing
the realize in set_default_size is correct, as that gets called a lot
(like from size_request).

I have committed the "clamp to screen" part of the patch, but that
isn't a full fix, because the dialog still comes up way too big on my
screen.


Fri Mar 12 23:37:15 2004  Soeren Sandmann  <sandmann@daimi.au.dk>

	* gtk/gtkfilechooserdialog.c
	(file_chooser_widget_default_realized_size_changed): Clamp the
	new size to the screen.

how does the new patch behave with large fonts + small screens?
I would think 800x600 with 16 point fonts or 1024x768 with 18 point
fonts would be a good worst-case target.  

So, I'm losing track of what this bug is about.  I'd really like to close this
and reopen bugs with individual issues.  Søren, what is the issue with clamping
it to the screen?

Mass changing gtk+ bugs with target milestone of 2.4.2 to target 2.4.4, as
Matthias said he was trying to do himself on IRC and was asking for help with. 
If you see this message, it means I was successful at fixing the borken-ness in
bugzilla :)  Sorry for the spam; just query on this message and delete all
emails you get with this message, since there will probably be a lot.

Yeah, let's close this down and open individual issues; it's too confusing.