Created attachment 367364 [details] [review]
sidebar: Remove focus from text in sidebar entry

Instead of 'has_focus' property, grab_focus_without_selecting fucntion is used to remove focus from text.

Review of attachment 367364 [details] [review]:

I think the commit subject is not quite correct here, rather than "Remove focus", I think it should mention not selecting the entry content, or something like that. Maybe "sidebar: Don't select text when focusing route entries".

::: src/sidebar.js
@@ +198,3 @@
 
+            this.connect('notify::child-revealed',
+                         routeEntry.entry.grab_focus_without_selecting.bind(routeEntry.entry));

This will actually trigger both when revealing and hiding the routing sidebar.
This has the effect that if the sidebar is revealed and then the user clicks on the search entry (for example) and then hides the sidebar (using the ctrl+d shortcut, not using the mouse as that would loose focus) then focus will shift over to the (now not visible) route from entry.
I think something like the following needs to be done:

this.connect('notify::child-revealed', (function() {
   if (this.child_revealed)
      routeEntry.entry.grab_focus_without_selecting();
}).bind(this));

Created attachment 367410 [details] [review]
sidebar: Deselect text when focusing route entries

made the required changes

Review of attachment 367410 [details] [review]:

::: src/sidebar.js
@@ +198,3 @@
 
+            this.connect('notify::child-revealed', (function(){
+                if(this.child_revealed)

Minor nitpick, should be a space before ( like:
if (condition..)

Attachment 367410 [details] pushed as 8e02777 - sidebar: Deselect text when focusing route entries

(In reply to Marcus Lundblad from comment #4)
> Review of attachment 367410 [details] [review] [review]:
> 
> ::: src/sidebar.js
> @@ +198,3 @@
>  
> +            this.connect('notify::child-revealed', (function(){
> +                if(this.child_revealed)
> 
> Minor nitpick, should be a space before ( like:
> if (condition..)

I pushed this with the formatting fix, so that it made it into the 3.27.90 release :) Thanks for patch!

(In reply to Marcus Lundblad from comment #6)
> (In reply to Marcus Lundblad from comment #4)
> > Review of attachment 367410 [details] [review] [review] [review]:
> > 
> > ::: src/sidebar.js
> > @@ +198,3 @@
> >  
> > +            this.connect('notify::child-revealed', (function(){
> > +                if(this.child_revealed)
> > 
> > Minor nitpick, should be a space before ( like:
> > if (condition..)
> 
> I pushed this with the formatting fix, so that it made it into the 3.27.90
> release :) Thanks for patch!

Thanks a lot for guiding me to solve this one. :)