On Sun, Oct 17, 2010 at 02:17:17PM +0530, Ishan Bansal wrote: > New variable text_status defined which could keep the track of the status of > text being entered and save it when activity is stopped.
What you do not say in this commit comment is how you have implemented the change. Having reviewed your code change, I see you are forcing the text entry to complete if the mouse leaves the drawing area while a text entry is in progress. In detail: 1. when the instance of the drawing area is created, you set your state variable to -1, 2. when a mouse down event occurs with the text tool selected, you set your state variable to 0, representing text entry in progress, 3. when the mouse moves with the text entry in progress, you set your state variable to 1, representing a mouse movement in progress during text entry, 4. when the mouse leaves the drawing area with the state variable set to 1, you attempt to complete the drawing of the text. Problems with this design: a. the text entry will be prematurely terminated if the user moves the mouse outside the drawing entry, which will be unexpected and annoying, b. the mouse does not move if the Ctrl/Q shortcut is used, therefore this code will not trigger, c. when system load is high, it is possible for a mouse leave event to arrive before a mouse move event, and so this code may not trigger. > Signed-off-by: Ishan Bansal<is...@seeta.in>, Anubhav > Aggarwal<anub...@seeta.in> There should be a space before < > --- > Area.py | 11 +++++++++++ > 1 files changed, 11 insertions(+), 0 deletions(-) > > v1->v2: Patch updated > > diff --git a/Area.py b/Area.py > index 2dca7da..a2db51b 100644 > --- a/Area.py > +++ b/Area.py > @@ -113,6 +113,8 @@ class Area(gtk.DrawingArea): > self.connect("leave_notify_event", self.mouseleave) > self.connect("enter_notify_event", self.mouseenter) > > + self.text_status = -1 > + Given my comments at the top of this message, you may not need to use the following comments if you abandon the design ... Please use a more appropriate variable name. "text_status" doesn't tell me anything about what it is for. "pending_text_state" may be more useful. You are using -1 to mean initialised, 0 to mean disarmed, and 1 to mean armed. These constants are used throughout your patch without further explanation. If you still need a state variable, please define constants, such as: PENDING_TEXT_STATE_RESET = -1 PENDING_TEXT_STATE_DISARMED = 0 PENDING_TEXT_STATE_ARMED = 1 The -1 state does not seem to be needed. > target = [('text/uri-list', 0, TARGET_URI)] > self.drag_dest_set(gtk.DEST_DEFAULT_ALL, target, > gtk.gdk.ACTION_COPY | gtk.gdk.ACTION_MOVE) > @@ -303,6 +305,7 @@ class Area(gtk.DrawingArea): > # text > if self.tool['name'] == 'text': > self.d.text(widget,event) > + self.text_status = 0 > > # This fixes a bug that made the text viewer get stuck in the canvas > elif self.estadoTexto is 1: (the above is in the mouse down event handler) > @@ -473,6 +476,10 @@ class Area(gtk.DrawingArea): > self.configure_line(self.line_size) > self.d.polygon(widget,coords,True,self.tool['fill'],"moving") > > + if self.tool['name'] == 'text': > + if self.text_status == 0: > + self.text_status = 1 > + > gtk.gdk.event_request_motions (event) > > def mouseup(self,widget,event): (the above is in the mouse move event handler) > @@ -571,6 +578,10 @@ class Area(gtk.DrawingArea): > size = self.tool['line size'] > widget.queue_draw_area(self.x_cursor-size, self.y_cursor-size, > size*2, size*2) > > + if self.tool['name'] == 'text': > + if self.text_status == 1: > + self.d.text(widget,event) > + > def mouseenter(self, widget, event): > if self.tool['name'] in ['pencil','eraser','brush','rainbow']: > self.drawing = False (the above is in the mouse leave event handler) -- James Cameron http://quozl.linux.org.au/ _______________________________________________ Sugar-devel mailing list Sugar-devel@lists.sugarlabs.org http://lists.sugarlabs.org/listinfo/sugar-devel