On Mon, Apr 21, 2008 at 10:01 AM, Tomeu Vizoso <[EMAIL PROTECTED]> wrote:
> On Sat, Apr 19, 2008 at 8:11 PM, Eben Eliason <[EMAIL PROTECTED]> wrote:
> > On Sat, Apr 19, 2008 at 6:41 AM, Tomeu Vizoso <[EMAIL PROTECTED]> wrote:
> > > On Wed, Apr 16, 2008 at 6:30 PM, Eben Eliason <[EMAIL PROTECTED]> wrote:
> > > > The former patch cut some comments that should have been cut (and now
> > > > have been) in the initial visual patch. This eliminates those parts
> > > > of the patch.
> > >
> > > + self._title_entry.props.widget.connect('focus-out-event',
> > > +
> > > self._title_entry_focus_out_event_cb)
> > >
> > > I'd indent the second line just two additional tabs to the right.
> >
> > I started there. The problem is that it's currently tabbed over as
> > far as it can go before breaking the 80 char boundary. Which style
> > convention should we break?
>
> Sorry, I explained badly. I meant this:
>
>
> self._title_entry.props.widget.connect('focus-out-event',
> self._title_entry_focus_out_event_cb)
>
> If you cannot make the indentation beautiful, then fall back to the
> simplest rule: two extra tabs (one may be confusing because of code
> blocks).
Done.
> > > + if event.key == hippo.KEY_RETURN:
> > > + self._set_title(entry.props.text)
> > > + self._title_entry.set_visible(False)
> > > + self._title.set_visible(True)
> > > + elif event.key == hippo.KEY_ESCAPE:
> > > + entry.props.text = self._title.props.text
> > > + self._title_entry.set_visible(False)
> > > + self._title.set_visible(True)
> > >
> > > I wonder if hardcoding the return and escape keys are really needed,
> > > or if gtk has a better way of doing this.
> >
> > Hmm, I'm not sure. I'll happily change this if someone has a better way.
>
> I think we want to apply the changes when the 'activate' and
> 'focus-out' signals are called. About when to undo the changes, seems
> like we need to listen for the escape key as you are doing.
I connected to the activate signal on the widget, as suggested.
>
> > > + self._title_entry.set_visible(False)
> > > + self._title.set_visible(True)
> > >
> > > These two lines are repeated after every time we end editing the
> > > title, can this duplication be removed?
> >
> > Sure. Any thoughts on an appropriate function name?
> > _restore_title_label, _title_edit_completed ?
>
> Can it be merged somehow inside _set_title()? And perhaps this method
> should be called instead _apply_title_change()?
Done.
>
> > > + def _set_title(self, title):
> > > + if title == '':
> > > + self._title_entry.props.text = self._title.props.text
> > >
> > > I guess that you don't want to let the user remove completely the
> > > title of an entry, can we make it more explicit?
> >
> > Indeed, that's the idea. Do you mean I should add a comment to this
> > effect above the conditional, or is there a way the code could read
> > more clearly itself?
>
> Hopefully the later. You are not doing anything specially complicated,
> so I would prefer if we could avoid the extra comment by making the
> intentions of the code more explicit.
There is now an _apply_title_change and a _cancel_title_change. The
condition in question simply calls cancel_title_change to clearly
indicate what's happening.
>
> > > self._title.props.text = self._format_title() + _('
> Activity')
> > > + self._title_entry.props.text = self._format_title() + _('
> > > Activity')
> > >
> > > We have a big problem here: https://dev.laptop.org/ticket/6875 .
> > > "Activity" should be a formatting thing, shouldn't get into the
> > > datastore.
> >
> > Yes, I noticed this problem while making this patch. I'm happy to see
> > there is already a ticket, as it was on my list to enter one for it
> > anyway. I think I'd actually recommend cutting the title formatting
> > completely, and simply depending upon the unique visual treatment to
> > identify the bundle for now. I'll think about it, and perhaps provide
> > a separate patch for it. Do you think it should go in along with this
> > patch?
>
> I think it could got in this patch if it's better for you, but
> attaching a more focused patch to the ticket may be better process.
I ignored this problem for this patch. I'll create a new patch to
clean this up.
Thanks!
- Eben
From 8773d5b4149e804a3a7808062c0a85d9a8513975 Mon Sep 17 00:00:00 2001
From: Eben Eliason <[EMAIL PROTECTED]>
Date: Sat, 19 Apr 2008 05:14:31 -0400
Subject: [PATCH] Add support for inline renaming of Journal entries
This provides support for renaming Journal entries directly
within the list view, by clicking on the title field.
---
collapsedentry.py | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 60 insertions(+), 0 deletions(-)
diff --git a/collapsedentry.py b/collapsedentry.py
index 5fe36d5..4b5efdf 100644
--- a/collapsedentry.py
+++ b/collapsedentry.py
@@ -28,6 +28,7 @@ from sugar.graphics.xocolor import XoColor
from sugar.graphics import style
from sugar.datastore import datastore
from sugar.graphics.roundbox import CanvasRoundBox
+from sugar.graphics.entry import CanvasEntry
from keepicon import KeepIcon
import misc
@@ -84,6 +85,10 @@ class CollapsedEntry(hippo.CanvasBox):
self._icon = self._create_icon()
self.append(self._icon)
+ self._title_entry = self._create_title_entry()
+ self.append(self._title_entry, hippo.PACK_EXPAND)
+ self._title_entry.set_visible(False)
+
self._title = self._create_title()
self.append(self._title, hippo.PACK_EXPAND)
@@ -135,8 +140,21 @@ class CollapsedEntry(hippo.CanvasBox):
xalign=hippo.ALIGNMENT_START,
font_desc=style.FONT_BOLD.get_pango_desc(),
size_mode=hippo.CANVAS_SIZE_ELLIPSIZE_END)
+ title.connect('button_release_event',
+ self.__title_button_release_event_cb)
return title
+ def _create_title_entry(self):
+ title_entry = CanvasEntry()
+ title_entry.set_background(style.COLOR_WHITE.get_html())
+ title_entry.props.widget.connect('focus-out-event',
+ self.__title_entry_focus_out_event_cb)
+ title_entry.props.widget.connect('activate',
+ self.__title_entry_activate_cb)
+ title_entry.connect('key-press-event',
+ self.__title_entry_key_press_event_cb)
+ return title_entry
+
def _create_buddies_list(self):
return BuddyList([], self._BUDDIES_COL_WIDTH)
@@ -225,6 +243,46 @@ class CollapsedEntry(hippo.CanvasBox):
self._jobject.resume()
return True
+ def __title_button_release_event_cb(self, button, event):
+ self._title.set_visible(False)
+ self._title_entry.set_visible(True)
+ self._title_entry.props.widget.grab_focus()
+
+ def __title_entry_focus_out_event_cb(self, entry, event):
+ self._apply_title_change(entry.props.text)
+
+ def __title_entry_activate_cb(self, entry):
+ self._apply_title_change(entry.props.text)
+
+ def __title_entry_key_press_event_cb(self, entry, event):
+ if event.key == hippo.KEY_ESCAPE:
+ self._cancel_title_change()
+
+ def _apply_title_change(self, title):
+ self._title_entry.set_visible(False)
+ self._title.set_visible(True)
+
+ if title == '':
+ self._cancel_title_change()
+ elif self._title.props.text != title:
+ self._title.props.text = title
+ self._jobject.metadata['title'] = title
+ self._jobject.metadata['title_set_by_user'] = '1'
+ datastore.write(self._jobject, update_mtime=False,
+ reply_handler=self._datastore_write_cb,
+ error_handler=self._datastore_write_error_cb)
+
+ def _cancel_title_change(self):
+ self._title_entry.props.text = self._title.props.text
+ self._title_entry.set_visible(False)
+ self._title.set_visible(True)
+
+ def _datastore_write_cb(self):
+ pass
+
+ def _datastore_write_error_cb(self):
+ logging.error('CollapsedEntry._datastore_write_error_cb: %r' % error)
+
def _detail_button_release_event_cb(self, button, event):
logging.debug('_detail_button_release_event_cb')
if not self._is_in_progress():
@@ -264,6 +322,7 @@ class CollapsedEntry(hippo.CanvasBox):
self._icon.props.fill_color=style.COLOR_TRANSPARENT.get_svg()
self._icon.props.stroke_color=style.COLOR_BLACK.get_svg()
self._title.props.text = self._format_title() + _(' Activity')
+ self._title_entry.props.text = self._format_title() + _(' Activity')
else:
if jobject.metadata.has_key('icon-color') and \
jobject.metadata['icon-color']:
@@ -272,6 +331,7 @@ class CollapsedEntry(hippo.CanvasBox):
else:
self._icon.props.xo_color = None
self._title.props.text = self._format_title()
+ self._title_entry.props.text = self._format_title()
self._buddies_list.set_model(self._decode_buddies())
--
1.5.3.3
_______________________________________________
Sugar mailing list
Sugar@lists.laptop.org
http://lists.laptop.org/listinfo/sugar
