GNOME Bugzilla – Bug 589337
"Number of items" doesn't activate OK button in "Create a GtkHBox" dialogue
Last modified: 2013-05-20 12:03:41 UTC
When creating an hbox (or vbox, or any other widget which uses a glade editable dialogue), pressing enter in the "Number of items" spinbutton (or whatever other widgets are in the dialog) doesn't activate the default action (the OK button).
When you create a Grid, the glade editable dialog has two spinbuttons - Rows and Columns. What should pressing enter do in this dialog? Should enter move to the next field or activate default action or ...?
Pressing ENTER should always trigger the "Create" button to activate. The fact that spin buttons are entries and entries handle the ENTER keystroke causes this to be tricky... but it definitely should trigger the "Create" button to activate.
Created attachment 182532 [details] [review] Activate popup dialog 'Create' action on enter key At first I thought the solution to this bug would be a one-liner in somewhere around glade_eprop_numeric_create_input(). But the eprops are also used by the right-hand Properties pane and probably elsewhere. I only wanted to 'activate default' in the dialog. The dialog code and glade_eprop_numeric_create_input code are quite a distance away in the run-time stack, the patch ended up a little larger.
Looks good, I like the general approach. Somethings need to change though, GladeEditorTable is "an" implementation of an editor widget, the editor widgets (GladeEditable implementaitons) can be created for every page type (currently this is an enum and represents the different notebook pages of the GladeEditor, however I would be happy if that could change so that the plugin backend can define page categories) ... anyway, long story short, what is shown in the query dialog is an editable widget created by the widget's adaptor for the QUERY type of page. Editables can embed editables... so the GladeEditorTable might not always be the one that is shown directly in the query dialog. A truly proper fix for this would be: - Add the set_activates_default() vfunc to the GladeEditable interface - Make all the editable implementations chain *down* into their embedded editors (i.e. GladeImageMenuItemEditor embeds a GladeEditorTable and a GladeImageEditor for the internal image, which in turn embeds another GladeEditorTable). What would be even nicer, is if all the editable implementations were to implement ->forall() to report their embedded editors (currently we have the same problem with ->set_show_name() which needs to be chained down to child editables, if we had a ->forall() vfunc for GladeEditable we could implement both ->set_show_name() and ->set_activates_default() as a default implementation of the GladeEditable and implementing classes would only have to implement the ->forall() method and implement the other ones optionally). Another, easier and less elegant way out, is if all the GladeEditable widgets stored the page type they were created for... and GladeEditorProperty also stored a pointer to the GladeEditable implementation it was created by... then GladeEPropText and GladeEpropNumeric could check the type of it's editor for QUERY and implicitly make it activate_default(). Also, I'm not fond of the way that glade_editor_property_set_activates_default() cases the eprop->input member. , Editor properties could potentially put a box there with an entry and a button (like a translatable text input for instance). So again on the glade-editor-property side, GladeEditorPropertyClass should be extended to have a ->set_activates_default() method as well, subclasses such as GladeEPropText and GladeEPropNumeric should implement them separately.
Craig, I believe I got an email from you some time ago... Believe it or not, we appreciate very much the time you have been spending on glade (the recent fixes you made do make a great difference), if this big change is too much, please feel free to move on to something else (I would have liked to take the time and refactor the editor apis myself however time did not permit unfortunately). Also, please come and hang out with us on gimpnet irc channel #glade3 if you have any questions or want to discuss anything about Glade.
The dialog buttons have default all right. The problem is spinbuttons eat the enter key.
Created attachment 194848 [details] [review] simplistic patch that works well enough for me Hmm, the property dialogs on creating a widget seem to be the only place where the property editor is used with a default button so the one-liner actually works for me. It would be nice to have the iterator and set the property explicitly with that but since nobody wrote that...
Committed Michal's patch... it's a bandaid but good enough at least for now. Sorry for the 2 year delay applying this, makes things a bit smoother. commit 28a9dd310aab5652505720160f6a9463c01dde2d Author: Tristan Van Berkom <tristan.van.berkom@gmail.com> Date: Mon May 20 21:00:36 2013 +0900 Patch up bug 589337 - Fix by Michal Suchanek <hramrach@gmail.com> This is a bandaid which fixes anoyance in dialogs which appear when creating variable widgets like HBox. The spin editor is used in multiple places but only htese dialogs have a default button so it is safe to set the spinbutton to activate default unconditionally. Does not seem to break anything.