[Bugs] #1586 UNSP: TreeView widget

[Sugar Labs Bugs]

#1586: TreeView widget
------------------------------------------+---------------------------------
    Reporter:  alsroot                    |          Owner:  erikos           
        Type:  enhancement                |         Status:  new              
    Priority:  Unspecified by Maintainer  |      Milestone:  0.88             
   Component:  sugar-toolkit              |        Version:  Git as of bugdate
    Severity:  Unspecified                |       Keywords:                   
Distribution:  Unspecified                |   Status_field:  Unconfirmed      
------------------------------------------+---------------------------------

Comment(by alsroot):

 Replying to [comment:6 tomeu]:
 > I'm not very happy with the design of SmoothTable and LazyModel. The
 other classes need to know about bin_rows, frames, cursors, models,
 sources, and I cannot figure out what more than half of those are. This is
 raising significantly the bar to hack on the journal without a high risk
 of adding regressions. In the existing lazy model, we encapsulate all the
 cache and readahead business inside model.py and it is not exposed to user
 classes, why do we must drop the benefits of such separation of concerns
 in order to get a thumbs view?

 what makes sense for SmootTable/TreeView I renamed and added comments

 > Doing a good class design for this is hard work, that's why gtk.TreeView
 sucks so much presently. There's an ongoing discussion about how to
 improve the TreeView class design in Gtk+ right now, see Kristian
 Rietveld's and Philip Van Hoof's blogs about it.

 well, in comparing to what Gnome needs, our case is very simple and since
 new widget is yet ready, proposed using(being very simple) could be useful
 choice

 > {{{
 >    2190 +        self._adj = None
 > }}}
 >
 > "adj" could mean a lot of different things.

 renamed to _adjustment

 > {{{
 >    2202 +        for i_ in range(rows + 2):
 > }}}
 >
 > '2' is a magical number, can we use a constant?

 added _SPARE_ROWS

 > {{{
 >    2207 +                cell.set_parent(self)
 >    2208 +                cell.size_allocate(gtk.gdk.Rectangle(-1, -1))
 >    2209 +                row.append(cell)
 > }}}
 >
 > One more circular dependency :( We should be spending more time
 designing the class model and less time coding and debugging.

 not sure how we can avoid gtk circular dependencies coding gtk
 applications :)

 > {{{
 >    2186 +    def __init__(self, rows, columns, new_cell, fill_in):
 > ...
 >    2205 +                cell = new_cell()
 > }}}
 >
 > How we use to do this is by having an abstract method that concrete
 subclasses can implement. In python is very easy and powerful to pass
 around references to methods but gets messy very quickly because it's hard
 to find where is defined the method that will be called. Every time you
 are tempted to pass a method reference, consider using instead an abstract
 method or a signal.

 well, new_cell, for example, could be a class or closure function
 so in case of using SmootTable in user code, he should create new class to
 redefine only these two functions, brrr...

 > {{{
 >    2214 +    def get_columns(self):
 > ...
 >    2219 +    def get_rows(self):
 > }}}
 >
 > I would expect that a method named like that would return a collection
 of all the rows or columns. What about calling them get_row_count and
 get_column_count?

 done

 > {{{
 >    2259 +    def goto(self, row):
 > }}}
 >
 > What about calling it scroll_to_row similarly to what gtk.TreeView
 already has?

 done

 > {{{
 >    2399 +            callocation = gtk.gdk.Rectangle(cell_x, cell_y)
 > }}}
 >
 > Can we avoid this abbreviation?

 done

 > {{{
 >    2407 +    def _get_head(self):
 > }}}
 >
 > Cannot get what this method does, can we have a more descriptive name
 for it?

 renamed to _get_upper_visible_row

 > {{{
 >    2412 +    def __adjustment_value_changed_cb(self, sender=None):
 > }}}
 >
 > What is sender here?

 done

 > {{{
 >    2427 +            class IndexedRow:
 > }}}
 >
 > Better define all classes always on the module scope level.

 done

 > {{{
 >    2475 +        uplimit = self._adj.upper - page
 > }}}
 >
 > upper_limit?

 done

-- 
Ticket URL: <http://bugs.sugarlabs.org/ticket/1586#comment:1>
Sugar Labs <http://sugarlabs.org/>
Sugar Labs bug tracking system