TableView widget

[alsroot]

 http://wiki.sugarlabs.org/go/Features/TableView_Widget

 http://git.sugarlabs.org/projects/sugar/repos/mainline/trees/tableview

[alsroot]

Replying to 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

[alsroot]

Replying to tomeu:

{{{
2625 +COLOR_BACKGROUND = style.COLOR_WHITE
2626 +COLOR_SELECTED = style.COLOR_TEXT_FIELD_GREY
}}}

Can we make these constants private?

removed, in former code, it was also used out of this file

{{{
2692 + canvas = hippo.Canvas()
}}}

Do we really need to use HippoCanvas? I think we should be reducing its usage, not increasing it.

the reason was using CanvasRoundBox

{{{
2696 + sel_box = CanvasRoundBox()
}}}

Can we avoid that abbr.?

done

{{{
2700 + cell = cell_class()
2701 + cell.table = self
}}}

One more circular reference :(

removed

{{{
2708 + canvas.table_view_cell_sel_box = sel_box
2709 + canvas.table_view_cell = cell
}}}

All class attributes should be set in the constructor, so in this case we would need a subclass of hippo.Canvas().

done

[alsroot]

Replying to alsroot:

Replying to tomeu:

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...

I changed SmartTable so it has the same ctor arguments as TableView
(SmartTable relies on having do_fill_in() method in passed class)

[sascha_silbe]

I'm a bit puzzled, this ticket is missing all comments from Tomeu. How could that happen? There were no email notifications being sent out for them either, yet alsroot managed to reply to them inside Trac...

[alsroot]

Replying to sascha_silbe:

I'm a bit puzzled, this ticket is missing all comments from Tomeu. How could that happen? There were no email notifications being sent out for them either, yet alsroot managed to reply to them inside Trac...

original ticket was #1226

[sascha_silbe]

Replying to alsroot:

Replying to sascha_silbe:

I'm a bit puzzled, this ticket is missing all comments from Tomeu. How could that happen? There were no email notifications being sent out for them either, yet alsroot managed to reply to them inside Trac...

original ticket was #1226

Ah, I see. Would be nice if you could state that and adjust or remove the Trac links as both Trac and I were confused. :)
Since we were having problems with the SPAM filter lately I suspected it to have failed again...

[alsroot]

The question about this widget, how to handle number of row/columns?

Existed code implements scheme with static number of visible columns/rows for widget, so after resizing table widget, number of cells will be preserved and only cells' sizes will be changed.

Another scheme is using static cell sizes and while resizing table widget, number of visible rows/columns will be changed.

And last scheme is leting cells set referable sizes and change number of visible column/rows correspondingly. Not sure if need such complex scheme somewhere in sugar.

[alsroot]

Replying to alsroot:

The question about this widget, how to handle number of row/columns?

Existed code implements scheme with static number of visible columns/rows for widget, so after resizing table widget, number of cells will be preserved and only cells' sizes will be changed.

Another scheme is using static cell sizes and while resizing table widget, number of visible rows/columns will be changed.

And last scheme is leting cells set referable sizes and change number of visible column/rows correspondingly. Not sure if need such complex scheme somewhere in sugar.

here, cell is Cell widget e.g. in case of Journal, there will Cell class which represent current row and not only one cellrenderer.

[tomeu]

I think we just need a table in which the columns have a preferred fixed width (number of pixels multiple of GRID_CELL) and the extra room is allocated in equal parts between the columns that have the EXPAND flag set. If there isn't enough room, each column is shrinked accordingly.

This should be easily doable in the Gtk+ sizing model, ping me if you find any difficulty.

[alsroot]

Replying to tomeu:

I think we just need a table in which the columns have a preferred fixed width (number of pixels multiple of GRID_CELL) and the extra room is allocated in equal parts between the columns that have the EXPAND flag set. If there isn't enough room, each column is shrinked accordingly.

This should be easily doable in the Gtk+ sizing model, ping me if you find any difficulty.

just to make clear if I understood it right, For the whole Table widget there is only one Cell class, so EXPAND flag could be for all cells(i.e. Table's property) or for each cell(each Cell instance). In last case, cells from two different rows could be aligned differently e.g in case of thumbs view:

+--------------------------------+
| thumb_1 | thumb_2              |
+--------------------------------+
|     thumb_3    |   thumb_4     |
+--------------------------------+

So, the question is having EXPAND flag for Table or Cell(makes code more complex and could be not so useful) classes.

[tomeu]

I'm a bit confused, I'm talking about columns but I'm not understanding what you mean by Cell there. If we had a 5x1000 table, would we have 5000 Cell instances?

[alsroot]

Table is not regular gtk.Container, user won't add subwidgets like to gtk.Box. User just pass Cell class and Table will dynamically create new Cell instance on demand and only for visible cells.

So in case of 5x1000 table, there will only <visible-columns>x<visible-rows> cells. While scrolling table, it will repaint visible cells to represent current "real" cell content.

[alsroot]

At the end Table behaves more like IconView rather then TreeView.
I just kept in mind simple widget but which will satisfy all our needs. For example with current Table we can build Journal with Cell which represent the entirely Journal's row as on Cell class(thus regular gtk widgets instead of cellrenders). Of course we can simulate TreeView by rewriting cellrender in widgets but do we really need such complexity.

[tomeu]

Replying to alsroot:

At the end Table behaves more like IconView rather then TreeView.
I just kept in mind simple widget but which will satisfy all our needs. For example with current Table we can build Journal with Cell which represent the entirely Journal's row as on Cell class(thus regular gtk widgets instead of cellrenders). Of course we can simulate TreeView by rewriting cellrender in widgets but do we really need such complexity.

Gotcha, then maybe this widget shouldn't be called TableView if it has only one column?

[alsroot]

Why only one column, one column only for Journal, for Thumbs view it will have several columns(since having only one thumb per row is not so useful). E.g. in case of thumbs, Cell class will contain subwidgets for image, label etc.

[alsroot]

so, what about letting users freedom to choose 1st of 2nd model from  http://bugs.sugarlabs.org/ticket/1586#comment:9 ?

[alsroot]

Replying to alsroot:

so, what about letting users freedom to choose 1st of 2nd model from  http://bugs.sugarlabs.org/ticket/1586#comment:9 ?

e.g.
* for Journal it will 1x<number-of-visible-rows> and 1st option
* for thumbs view, it will be min sizes for cell and number of visible columns/rows according to current Table's size

[alsroot]

or having 1st/2nd options separately for columns-cumber/cell-width and for rows-number/cell-height

[tomeu]

Replying to alsroot:

Replying to alsroot:

so, what about letting users freedom to choose 1st of 2nd model from  http://bugs.sugarlabs.org/ticket/1586#comment:9 ?

e.g.
* for Journal it will 1x<number-of-visible-rows> and 1st option
* for thumbs view, it will be min sizes for cell and number of visible columns/rows according to current Table's size

Sounds good, though I will be able to better understand it with two code snippets that set up the view in each case.

[alsroot]

(examples assume that TableView is model-less class, so we set number of "real" columns/rown manually)

1st option:

table = TableView(cell_class=Cell, cell_numbers=(2,2))
table.column_numbers = 10
table.row_numbers = 10

will produce:

+-------------------+-------------------+
|                   |                   |
|  cell_1(1)        |     cell_2(1)     |
|                   |                   |
+-------------------+-------------------+
|                   |                   |
|  cell_1(2)        |     cell_2(2)     |
|                   |                   |
+-------------------+-------------------+

2st option:

table = TableView(cell_class=Cell, cell_sizes=(1,1))
table.column_numbers = 10
table.row_numbers = 10

will produce:

+---------+---------+---------+---------+
|cell_1(1)|cell_2(1)|cell_3(1)|cell_4(1)|
+---------+---------+---------+---------+
|cell_1(2)|cell_2(2)|cell_3(2)|cell_4(2)|
+---------+---------+---------+---------+
|cell_1(3)|cell_2(3)|cell_3(3)|cell_4(3)|
+---------+---------+---------+---------+
|cell_1(4)|cell_2(4)|cell_3(4)|cell_4(4)|
+---------+---------+---------+---------+

[alsroot]

I've pushed initial refactoring commit to tableview branch
it's not yet fully implemented but it can show how refactoring is going

 http://git.sugarlabs.org/projects/sugar/repos/mainline/trees/tableview

[sascha_silbe]

Why don't we put this in sugar-toolkit (sugar.graphics)? There are probably other parts of code (and even activities once the new API is stable enough) that could benefit from such a widget.

[alsroot]

Replying to sascha_silbe:

Why don't we put this in sugar-toolkit (sugar.graphics)? There are probably other parts of code (and even activities once the new API is stable enough) that could benefit from such a widget.

we need to stabilize API and for me there was another reason - I'm going to reimplement this widget for Vala based toolkit

[alsroot]

For now, it should be full functional journal, list view and thumbs view except DND(I posted a topic for new designers meeting  http://wiki.sugarlabs.org/go/Design_Team/0.88_Meeting#Drag_icon) and extended gtk.TreeView features like inplace search(not sure if we need them, at least right now).

I think later coding could be useful only after merging this part.

[tomeu]

The journal is not starting for me:

  File "/home/tomeu/sugar-jhbuild/install/lib/python2.6/site-packages/jarabe/journal/view.py", line 69, in <lambda>
    VIEW_LIST: lambda: self._view_new(ListView),
  File "/home/tomeu/sugar-jhbuild/install/lib/python2.6/site-packages/jarabe/journal/view.py", line 193, in _view_new
    view = view_class()
TypeError: __init__() takes exactly 2 arguments (1 given)

As an aside, I think this case illustrates the point of using python's dynamic capabilities sparingly: it takes longer to discover what init method the stack trace refers to. In order to know it, I have to trace the code and see which are the possible values of the variable "view_class".

[alsroot]

I've pushed new commit w/ fix and w/o cell_class

[tomeu]

Moving to 0.90 as per irc

[tomeu]

Aleksey, is this still in line with your plans for the journal?

[alsroot]

I'm closing this ticket since it was already implemented in polyol/gui vala library and could be used from there. Not sure if it is useful to support two parallel implementations.