[Sugar-devel] Browse feature review for Google Code In - Clear URL bar when Escape is pressed.

James Cameron quozl at laptop.org
Tue Nov 19 02:50:31 EST 2013


Thanks.

Small problem:

The narrative is all on one line.  The convention is to use a summary
in first line, then a gap, then the explanation.  The reason for the
convention is that git and associated web tools use the first line as
a summary of the patch.

For example:

	Clear URL on Escape key

	Problem: there was no shortcut for clearing the URL entry box.
	The logical choice is the Escape key, which closes the history
	search results window.

	Solution: clear the URL when the Escape key is pressed in the
	URL entry box.

	Google Chrome and Firefox handles this binding in a new tab
	only; Browse now handles this binding in all tabs.

	Fixes #3954.

As you possibly know, you can do this change quickly using "git commit
--amend".

On Tue, Nov 19, 2013 at 01:07:15PM +0530, Sai Vineet wrote:
> Here's the modified patch as per James Cameron's requirements.
> 
> 
> On Tue, Nov 19, 2013 at 3:24 AM, James Cameron <quozl at laptop.org> wrote:
> 
>     Tested-by: James Cameron <quozl at laptop.org>
> 
>     - it works for me, discarding both the URL text, hiding the X delete
>       icon, and hiding the history search results.
> 
>     - the single key press solution is acceptable to me, having to press
>       escape twice would be irritating,
> 
>     Reviewed-by: James Cameron <quozl at laptop.org>
> 
>     - the patch narrative might have more text in subsequent lines
>       describing the problem and how it is solved, with a reference to the
>       Sugar Labs ticket number.
> 
>     On Tue, Nov 19, 2013 at 12:53:12AM +0530, Sai Vineet wrote:
>     > I've attached the patch. It is a one liner.
>     > The mentors in GCI, namely Walter Bender and Gonzalo Odiard, told me that
>     this
>     > review may take time, so I should move on the other tasks after
>     unclaiming this
>     > task.
>     >
>     > Please review this fast. Thank you!
> 
>     > From 79a02c411594eacf4af2d796e6c1acfb7a241317 Mon Sep 17 00:00:00 2001
>     > From: Sai Vineet <saivineet89 at gmail.com>
>     > Date: Mon, 18 Nov 2013 23:56:40 +0530
>     > Subject: [PATCH 1/1] Added clear URL bar with Escape key feature
>     >
>     > ---
>     >  webtoolbar.py |    1 +
>     >  1 file changed, 1 insertion(+)
>     >
>     > diff --git a/webtoolbar.py b/webtoolbar.py
>     > index b6ad558..2fb98f5 100644
>     > --- a/webtoolbar.py
>     > +++ b/webtoolbar.py
>     > @@ -231,6 +231,7 @@ class WebEntry(iconentry.IconEntry):
>     >              return True
>     >          elif keyname == 'Escape':
>     >              self._search_window.hide()
>     > +            self.props.text = ''
>     >              return True
>     >          return False
>     >
>     > --
>     > 1.7.9.5
>     >
> 
>     > _______________________________________________
>     > Sugar-devel mailing list
>     > Sugar-devel at lists.sugarlabs.org
>     > http://lists.sugarlabs.org/listinfo/sugar-devel
> 
> 
>     --
>     James Cameron
>     http://quozl.linux.org.au/
> 
> 

> From 1791c7bd5359ea07b9a1fb7b0dc2c6985443d6a3 Mon Sep 17 00:00:00 2001
> From: Sai Vineet <saivineet89 at gmail.com>
> Date: Tue, 19 Nov 2013 13:05:06 +0530
> Subject: [PATCH 1/1] Enhancement : Clear URL Entry when Escape is pressed in
>  Browse The ticket is here -
>  http://bugs.sugarlabs.org/ticket/3954 The feature
>  request was that the URL should be cleared when the URL
>  bar is in focus and Escape is pressed. I just added a
>  line to clear text in webtoolbar.py's WebEntry class's
>  keypress handler. One elif  block was already there for
>  the Escape key and it hided the suggestions box.
> 
> ---
>  webtoolbar.py |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/webtoolbar.py b/webtoolbar.py
> index b6ad558..2fb98f5 100644
> --- a/webtoolbar.py
> +++ b/webtoolbar.py
> @@ -231,6 +231,7 @@ class WebEntry(iconentry.IconEntry):
>              return True
>          elif keyname == 'Escape':
>              self._search_window.hide()
> +            self.props.text = ''
>              return True
>          return False
>  
> -- 
> 1.7.9.5
> 


-- 
James Cameron
http://quozl.linux.org.au/


More information about the Sugar-devel mailing list