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 779586 - set initial focus on the new recipe page
set initial focus on the new recipe page
Status: RESOLVED FIXED
Product: recipes
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Recipes maintainer(s)
Recipes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2017-03-04 18:48 UTC by Matthias Clasen
Modified: 2017-03-12 17:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Set initial focus on the "Name Entry" (4.27 KB, patch)
2017-03-11 20:11 UTC, Maithili Bhide
none Details | Review
Set initial focus on the "Name Entry" (1.20 KB, patch)
2017-03-12 04:36 UTC, Maithili Bhide
none Details | Review
Set initial focus on the "Name Entry" (1.22 KB, patch)
2017-03-12 17:06 UTC, Maithili Bhide
committed Details | Review

Description Matthias Clasen 2017-03-04 18:48:51 UTC
When clicking the "New Recipe" button, I end up on the edit page for a new recipe, but when I start typing, nothing happens, because the keyboard focus is not in a useful place.

What I would expect to happen is that the keyboard focus shold be on the Name: entry, letting me start typing the recipe name without further ado.

The GTK+ api to investigate forthis is gtk_widget_grab_focus()
Comment 1 Maithili Bhide 2017-03-04 18:51:24 UTC
Working on it
Comment 2 Matthias Clasen 2017-03-04 18:51:36 UTC
Another thing to keep in mind: the 'new recipe' page is the same as the 'edit existing recipe' page, so your changes need to work for (or at least not negatively affect) that case too.
Comment 3 Maithili Bhide 2017-03-11 20:11:59 UTC
Created attachment 347725 [details] [review]
Set initial focus on the "Name Entry"

Sets keyboard input focus on the "Name Entry" field while adding a new recipe as well as while editing an existing recipe.
Comment 4 Matthias Clasen 2017-03-11 21:59:07 UTC
Review of attachment 347725 [details] [review]:

Thanks for the patch. First the positive: it clearly works! It think it does a bit more work than necessary, though. See detailed review.

::: src/gr-edit-page.c
@@ +1823,3 @@
 
+        gtk_widget_grab_focus(page->name_entry);
+

Formatting minutiae: please put a space between the function name and the opening (.

@@ +2025,3 @@
 
+        gtk_widget_grab_focus(page->name_entry);
+

Here too.

@@ +2064,3 @@
 
+        gtk_widget_grab_focus(page->name_entry);
+

I don't think this call is necessary. It should be sufficient to grab the focus when we come to the page, which happens either via gr_edit_page_clear() (for a new recipe) or via gr_edit_page_edit() for an existing recipe).

@@ +2108,3 @@
                               "mtime", g_date_time_new_now_utc (),
                               NULL);
+                gtk_widget_grab_focus(page->name_entry);

Don't think this one is needed either.

@@ +2143,3 @@
                 ret = gr_recipe_store_add_recipe (store, recipe, &error);
                 g_set_object (&page->recipe, recipe);
+                gtk_widget_grab_focus(page->name_entry);

And neither is this.

I removed those three extra calls, and things still worked fine in my testing.

::: src/gr-edit-page.ui
@@ +296,3 @@
                         <property name="width-chars">30</property>
+                        <property name="has-focus">1</property>
+                        <property name="can-focus">1</property>

has-focus is a 'transient' property that tracks where the focus is at runtime. Setting it from a ui file won't do much good.

can-focus potentially makes sense, but I don't think we actually want the focus to end up on the page itself, ever, so it may be best not to do this.

::: src/gr-window.c
@@ -181,3 @@
 
-        gtk_widget_grab_focus(window);
-

I don't think this is needed - grabbing the focus directly to the entry inside the edit page should work fine without this.

::: src/gr-window.ui
@@ +4,3 @@
   <template class="GrWindow" parent="GtkApplicationWindow">
     <property name="resizable">1</property>
+    <property name="has-focus">1</property>

See my comment on has-focus in the other ui file
Comment 5 Maithili Bhide 2017-03-12 04:36:52 UTC
Created attachment 347734 [details] [review]
Set initial focus on the "Name Entry"

Sets keyboard input focus on the "Name Entry" field while adding a new recipe as well as while editing an existing recipe.
Comment 6 Matthias Clasen 2017-03-12 12:19:44 UTC
Review of attachment 347734 [details] [review]:

looks good now, thanks!
Comment 7 Matthias Clasen 2017-03-12 12:24:24 UTC
Review of attachment 347734 [details] [review]:

But there's actually a little more work to do, as I realized when I tried to apply the patch: it doesn't cleanly, since the underlying code has shifted a bit since you started working on this. I could fix this up, but I think it is more instructional if you do it yourself, since this kind of thing happens all the time in practice.

Also, there is some extra whitespace at the end of your newly added lines and in the empty lines around it. Can you remove that too, please ?
Comment 8 Maithili Bhide 2017-03-12 17:06:11 UTC
Created attachment 347761 [details] [review]
Set initial focus on the "Name Entry"