After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769608 - [PATCHES] ComboBox only ever upsizes when the model changes, but it should also shrink if appropriate
[PATCHES] ComboBox only ever upsizes when the model changes, but it should al...
Status: RESOLVED OBSOLETE
Product: gtk+
Classification: Platform
Component: Widget: GtkComboBox
3.22.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-08-07 20:04 UTC by Daniel Boles
Modified: 2018-05-02 17:22 UTC
See Also:
GNOME target: ---
GNOME version: 3.21/3.22


Attachments
gcc `pkg-config --cflags gtk+-3.0` -std=c11 test.c `pkg-config --libs gtk+-3.0` (2.50 KB, text/x-csrc)
2016-08-07 20:04 UTC, Daniel Boles
  Details
screencast of compiled test.c (232.45 KB, video/webm)
2016-08-07 20:09 UTC, Daniel Boles
  Details
[PATCH 1/3] GtkCellView: add shrink_to_fit_model() (2.70 KB, patch)
2016-12-04 00:53 UTC, Daniel Boles
none Details | Review
[PATCH 2/3] GtkComboBox: Add private shrink_cell_view() (1.61 KB, patch)
2016-12-04 00:54 UTC, Daniel Boles
needs-work Details | Review
[PATCH 3/3] ComboBox and Text: Shrink cell view to fit model (2.84 KB, patch)
2016-12-04 00:54 UTC, Daniel Boles
needs-work Details | Review
[PATCH 1/3] GtkCellView: add shrink_to_fit_model() (2.53 KB, patch)
2016-12-04 01:07 UTC, Daniel Boles
needs-work Details | Review

Description Daniel Boles 2016-08-07 20:04:08 UTC
Created attachment 332893 [details]
gcc `pkg-config --cflags gtk+-3.0` -std=c11 test.c `pkg-config --libs gtk+-3.0`

As in the title and demonstrated by the attached C program.

If you populate a ComboBox with some options whose values as rendered on the button are narrow (short text and/or small image), then change to a model with any wider value(s) - the ComboBox resizes _up_ (preferred_size) to fit the new most-wide option. That's great!

But if you change to a model whose options are all narrower than the current model... the ComboBox's Button doesn't size back _down_ appropriately. It seems like the resize does a max() relative to the current size of the Button, rather than purely among the new model's options. End result: the new width is the max of all options the CB has ever seen. Manually setting a size_request can't shrink it.

This leads to inconsistencies when using ComboBoxes with dynamically populated models of options, e.g.:
 * A CB that had a nice narrow, exactly sized Button at one point in the program can later have a menu that's far too wide - based on options that are no longer there, from a previous use of the CB with a totally different model.
 * The corollary is that it leads to some instances of the CB having big gaps of empty space on either side of the displayed value, and others not.

Can this be sorted so that the ComboBox's Button is always appropriately resized to fit its current model, instead of taking the max of all options it's ever had?

Thanks.
Comment 1 Daniel Boles 2016-08-07 20:09:53 UTC
Created attachment 332894 [details]
screencast of compiled test.c
Comment 2 Daniel Boles 2016-08-28 11:34:03 UTC
Correct me if I'm wrong: I have since found that seemingly, what I want could be achieved via gtk_cell_area_context_reset()

> https://developer.gnome.org/gtk3/stable/GtkCellAreaContext.html#gtk-cell-area-context-reset
>
> Resets any previously cached request and allocation data.
>
> When underlying GtkTreeModel data changes its important to reset the context if the content size is allowed to shrink. If the content size is only allowed to grow [...]

This makes it sound very desirable that users be able to shrink their widgets to match the new contents of the associated models. Exactly what I'm trying to do. .. if only ListStore did this for me, or I had any other way to get at the GtkCellAreaContext used by my ComboBox via the CellArea it uses in its Button.


But I cannot find a way to do so. While we can get a GtkCellArea from the ComboBox, in the CellArea, we only have these:

> GtkCellAreaContext * 	gtk_cell_area_create_context ()
> GtkCellAreaContext * 	gtk_cell_area_copy_context ()

which only let us get a new Context, or copy the existing one. There is no way to take an editable reference to the existing one, or set another in its place. I feel like there probably should be.


So, is there any way to navigate the widget hierarchy from a ComboBox or other cell-having widget, and get a pointer to the Context in order to perform the reset() that is so heartily recommended above... or would users wanting a shrink-able ComboBox currently have to derive their own in order to get at this?

If the latter is true, could a little piece of API be added to CellArea et al to enable users to reset() and hence shrink their CellAreas, without having to reinvent the rest of the wheel just to use this one spoke? Or at least, any other Gtk classes relying on CellAreaContext should expose a reset() method.


Thanks!
Comment 3 Daniel Boles 2016-08-28 11:43:37 UTC
ACtually, the suggestions above probably greatly overcomplicate things. In the case I originally outlined, it seems like it would suffice to have any action that replaces the model, i.e. set_model(), run the reset() internally, without having to expose this as API.
Comment 4 Daniel Boles 2016-08-28 11:46:02 UTC
...in fact, no, ComboBox only sees CellArea, just like us, so it would have no way to call reset(). It'd either need a way to do that, or the shrinking would have to be achieved some other way.
Comment 5 Daniel Boles 2016-12-03 22:29:06 UTC
Toggling CellView::fit-model to FALSE and back to TRUE after setting the new model seems to force the CellView and hence ComboBox to be 'shrunk' to the proper appropriate size. I feel like if fit-model is left TRUE, then when setting model, whatever's causing the shrinking in the toggling case should also be applied then.
Comment 6 Daniel Boles 2016-12-03 22:53:19 UTC
(In reply to Daniel Boles from comment #5)
> Toggling CellView::fit-model to FALSE and back to TRUE after setting the new
> model seems to force the CellView and hence ComboBox to be 'shrunk' to the
> proper appropriate size.

...ONLY if done via the Inspector. Doing this in code doesn't seem to make any difference. Does the Inspector do some additional shenanigans?
Comment 7 Daniel Boles 2016-12-04 00:53:24 UTC
Created attachment 341332 [details] [review]
[PATCH 1/3] GtkCellView: add shrink_to_fit_model()

I've sketched up a possible implementation and would appreciate comments on sense, naming, etc.

This works for both ComboBox with a list model, and ComboBoxText, for the cases I've tried at least.

> This function will be used by GtkComboBox in the next commits.
> It is also called here in gtk_cell_view_set_model().
> 
> By checking if fit-model is TRUE and, if so, calling
> gtk_cell_area_context_reset, any preferred size that has been
> accumulated based on previous contents of the model is cleared.
Comment 8 Daniel Boles 2016-12-04 00:54:10 UTC
Created attachment 341333 [details] [review]
[PATCH 2/3] GtkComboBox: Add private shrink_cell_view()

> This will be used to ensure that if the CellView has fit-model == TRUE,
> fitting is also applied when the model is changed or unset. This was not
> previously the case, so a ComboBox could only upsize and therefore would
> be oversized for smaller models if a larger one had previously been set.
> 
> Needless to say, if the ComboBox does not currently have an internal
> CellView, this function is a no-op.
Comment 9 Daniel Boles 2016-12-04 00:54:40 UTC
Created attachment 341334 [details] [review]
[PATCH 3/3] ComboBox and Text: Shrink cell view to fit model

> ...in various places, using the new function from gtkcomboboxprivate.h.
> The existing code already handled upsizing when the model was changed,
> so this just needs to add shrinks when rows are modified or removed.
Comment 10 Daniel Boles 2016-12-04 01:07:03 UTC
Created attachment 341335 [details] [review]
[PATCH 1/3] GtkCellView: add shrink_to_fit_model()

Drop an off-topic paragraph from the doc comment
Comment 11 Daniel Boles 2017-02-21 13:12:05 UTC
Review of attachment 341335 [details] [review]:

This should be a _gtk_combo_box internal method, and the checks on cell_view are unnecessary as the callers already do that.
Comment 12 Daniel Boles 2017-02-21 13:12:43 UTC
Review of attachment 341333 [details] [review]:

The check of priv->cell_view is unnecessary, as the callers already do that
Comment 13 Daniel Boles 2017-02-21 13:17:35 UTC
Review of attachment 341335 [details] [review]:

correction:

::: gtk/gtkcellview.c
@@ +1604,3 @@
+ *
+ * Since: 3.22.5
+ */

This should probably be a private function for widget implementations only, which would mean the documentation is superfluous.

::: gtk/gtkcellview.h
@@ +114,3 @@
+GDK_AVAILABLE_IN_3_22
+void              gtk_cell_view_shrink_to_fit_model     (GtkCellView     *cell_view);
+

This should probably be a private function, in a new gtkcellviewprivate.h.
Comment 14 Daniel Boles 2017-02-21 13:20:09 UTC
Review of attachment 341334 [details] [review]:

::: gtk/gtkcombobox.c
@@ +2897,3 @@
+          GtkCellView *cell_view = GTK_CELL_VIEW (priv->cell_view);
+
+          gtk_cell_view_shrink_to_fit_model (cell_view);

This should call gtk_combo_box_shrink_cell_view(), rather than reimplementing it.

@@ +2938,3 @@
+          GtkCellView *cell_view = GTK_CELL_VIEW (combo_box->priv->cell_view);
+
+          gtk_cell_view_shrink_to_fit_model (cell_view);

This should call gtk_combo_box_shrink_cell_view(), rather than reimplementing it.
Comment 15 GNOME Infrastructure Team 2018-05-02 17:22:21 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gtk/issues/652.