Merge lp:~nicolozilio/software-center/lp842706 into lp:software-center

Proposed by Nicolò Zilio
Status: Rejected
Rejected by: dobey
Proposed branch: lp:~nicolozilio/software-center/lp842706
Merge into: lp:software-center
Diff against target: 62 lines (+23/-11)
1 file modified
softwarecenter/ui/gtk3/widgets/searchentry.py (+23/-11)
To merge this branch: bzr merge lp:~nicolozilio/software-center/lp842706
Reviewer Review Type Date Requested Status
Kiwinote Needs Fixing
dobey Needs Fixing
Michael Vogt Pending
Review via email: mp+142008@code.launchpad.net

Description of the change

Make a mistake during resolving the conflict.
This is a new branch with all updated

To post a comment you must log in.
Revision history for this message
dobey (dobey) wrote :

Hi Nicolo, can you please sign the Contributor Agreement by filling the on-line form at http://www.canonical.com/contributors and list the Canonical Contact as Roberto Alsina so that we may be able to accept patches from you?

Also, please remove the changes to debian/changelog in your branches, as we'll add them when we release, changing them in proposals like this creates unnecessary conflicts, and the version string and your e-mail address are not valid, there is a missing space character between the asterisk and text of that line, and you don't need to list your name twice. The [Name] entries are only used when someone else is listing the changes in the package.

Thanks.

review: Needs Fixing
3285. By Nicolò Zilio

Updated debian/chagelog

Revision history for this message
Nicolò Zilio (nicolozilio) wrote :

> Also, please remove the changes to debian/changelog in your branches, as we'll add them when we release, changing them
> in proposals like this creates unnecessary conflicts, and the version string and your e-mail address are not valid,
> there is a missing space character between the asterisk and text of that line, and you don't need to list your name
> twice. The [Name] entries are only used when someone else is listing the changes in the package.

Sorry, but I have to remove all changes leaving debian/changelog as the original branch one?

Thanks
Nicolò

3286. By Nicolò Zilio

Updated debian/chagelog & bugfix code

Revision history for this message
dobey (dobey) wrote :

On Mon, 2013-01-14 at 16:25 +0000, Nicolò Zilio wrote:
> Sorry, but I have to remove all changes leaving debian/changelog as the original branch one?

Yes, please leave debian/changelog unchanged in your merge proposals.
Thanks.

3287. By Nicolò Zilio

Updated debian/chagelog & bugfix code

Revision history for this message
Nicolò Zilio (nicolozilio) wrote :

Updated the branch eliminating changes in debian/changelog.
Hoping to help.
Tell me for other problems about that branch.

Revision history for this message
Kiwinote (kiwinote) wrote :

Hi Nicolò!

Thanks a lot for your branch - I took a quick initial look at it and bumped into the following issues:
- previously, clicking the magnifying glass icon selected the search text, this no longer works (in LTR mode),
- when using software-center in RTL mode (./software-center --force-rtl) the clear button no longer works,
- when pressing the clear button, then dragging the mouse to the grey area just below the search box, then releasing the mouse button the clear action is still triggered.

From a more general point of view, hard coding values like 17 and 192 is a little random, as the height / width of a widget is variable rather than constant. Also we try to follow the pep8 guidelines for style formatting (see [0]) - you will note that line 83 of searchentry.py is longer than 79 characters - in this case we prefer to split the clause over two lines.

Hope this helps, and thanks a lot for helping to improve software-center!

[0] http://www.python.org/dev/peps/pep-0008/

review: Needs Fixing

Unmerged revisions

3287. By Nicolò Zilio

Updated debian/chagelog & bugfix code

3286. By Nicolò Zilio

Updated debian/chagelog & bugfix code

3285. By Nicolò Zilio

Updated debian/chagelog

3284. By Nicolò Zilio

*lp:~nicolozilio/software-center/lp842706:
- fix claning bug on SearchEntry.py (LP: #842706)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwarecenter/ui/gtk3/widgets/searchentry.py'
2--- softwarecenter/ui/gtk3/widgets/searchentry.py 2012-11-28 15:54:20 +0000
3+++ softwarecenter/ui/gtk3/widgets/searchentry.py 2013-01-14 17:36:22 +0000
4@@ -42,7 +42,8 @@
5 """
6 Gtk.Entry.__init__(self)
7 self.set_width_chars(25)
8- self.set_size_request(0, em(1.7))
9+ self.set_size_request(230, em(1.7))
10+ self.is_pressed = False
11
12 if not icon_theme:
13 icon_theme = Gtk.IconTheme.get_default()
14@@ -51,6 +52,7 @@
15 self._on_changed)
16
17 self.connect("icon-press", self._on_icon_pressed)
18+ self.connect("icon-release", self._on_icon_released)
19
20 self.set_icon_from_icon_name(Gtk.EntryIconPosition.PRIMARY,
21 'edit-find-symbolic')
22@@ -66,20 +68,30 @@
23 self._redo_stack = []
24
25 def _on_icon_pressed(self, widget, icon, mouse_button):
26+ self.is_pressed = True
27+
28+ def _on_icon_released(self, widget, icon, mouse_button):
29 """
30 Emit the terms-changed signal without any time out when the clear
31 button was clicked
32- """
33- if icon == Gtk.EntryIconPosition.SECONDARY:
34- # clear with no signal and emit manually to avoid the
35- # search-timeout
36- self.clear_with_no_signal()
37- self.grab_focus()
38- self.emit("terms-changed", "")
39+ """
40+ # things for checking if user have clicked (press and release) and
41+ # if the mouse is in the SearchEntry area
42+ (x, y) = self.get_pointer()
43+ rect = self.get_allocation()
44
45- elif icon == Gtk.EntryIconPosition.PRIMARY:
46- self.select_region(0, -1)
47- self.grab_focus()
48+ if self.is_pressed == True and x > 192 and x < rect.width and y > 17 and y < rect.height:
49+ if icon == Gtk.EntryIconPosition.SECONDARY:
50+ # clear with no signal and emit manually to avoid the
51+ # search-timeout
52+ self.clear_with_no_signal()
53+ self.grab_focus()
54+ self.emit("terms-changed", "")
55+ elif icon == Gtk.EntryIconPosition.PRIMARY:
56+ self.select_region(0, -1)
57+ self.grab_focus()
58+
59+ self.is_pressed = False
60
61 def undo(self):
62 if len(self._undo_stack) <= 1: