Issue 370861002: tryton-env: Request a column size recomputation on treeview realization (Closed)
DescriptionPatch Set 1 #
Total comments: 6
Patch Set 2 : Fix remarks #
Total comments: 23
Patch Set 3 : Fix remarks #Patch Set 4 : Use bytes instead of str #Patch Set 5 : Fix arguments passed to underlying setter #Patch Set 6 : Use queue_resize when view hasn't been realized yet #
Total comments: 2
Patch Set 7 : Use scope to store realized status #Patch Set 8 : Fix method signature & treeview getter #
Total comments: 1
Patch Set 9 : Fix function call #MessagesTotal messages: 39
Review's title does not follow the convention: '^(?P<repository>[A-Za-z_][\w\.-]+)(?P<version> [0-9.]+)?:' URL: https://codereview.tryton.org/370861002
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/397291002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/397291002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:53: os.path.join(PIXMAPS_DIR, 'empty.png')) Could we use svg to reduce the size? https://codereview.tryton.org/370861002/diff/397291002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:64: cell.set_property('text', '') Should not we have at least a single space. https://codereview.tryton.org/370861002/diff/397291002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:71: pixbuf = common.resize_pixbuf(EMPTY_IMG, height, width) I think we should have some cache for it to avoid resize for every row.
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/397291002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/397291002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:53: os.path.join(PIXMAPS_DIR, 'empty.png')) On 2022/02/10 19:10:20, ced wrote: > Could we use svg to reduce the size? Done. https://codereview.tryton.org/370861002/diff/397291002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:64: cell.set_property('text', '') On 2022/02/10 19:10:21, ced wrote: > Should not we have at least a single space. Indeed https://codereview.tryton.org/370861002/diff/397291002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:71: pixbuf = common.resize_pixbuf(EMPTY_IMG, height, width) On 2022/02/10 19:10:20, ced wrote: > I think we should have some cache for it to avoid resize for every row. Done.
Sign in to reply to this message.
Review's title does not follow the convention: '^(?P<repository>[A-Za-z_][\w\.-]+)(?P<version> [0-9.]+)?:' URL: https://codereview.tryton.org/370861002
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/data/pix... File tryton/tryton/data/pixmaps/empty.svg (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/data/pix... tryton/tryton/data/pixmaps/empty.svg:5: width="64.000000pt" height="64.000000pt" viewBox="0 0 64.000000 64.000000" should not it be 24 as other icons? https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/data/pix... tryton/tryton/data/pixmaps/empty.svg:9: </metadata> I think it should be stripped using svgo: https://www.npmjs.com/package/svgo https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/data/pix... tryton/tryton/data/pixmaps/empty.svg:13: </svg> It seems this is just enough: <svg version="1.0" xmlns="http://www.w3.org/2000/svg" width="24" height="24"/> I'm wondering if it really need to be in a file. Maybe it could be stored in common as pixbuf. https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:64: cell = args[1] Maybe the wrapper could use the signature: (self, cell, *args, *kwargs) https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:66: cell.set_property('text', ' ') Is it enough when the content once realized is bigger than 1 char? https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:69: _, width, height = Gtk.IconSize.lookup(Gtk.IconSize.MENU) Maybe the width and height could be defined also for this Cells. This would avoid to duplicate which IconSize to use for each one. https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:73: key = (id(self), width, height) Why should it be per id? https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:80: not needed.
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/data/pix... File tryton/tryton/data/pixmaps/empty.svg (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/data/pix... tryton/tryton/data/pixmaps/empty.svg:13: </svg> On 2022/02/16 14:07:18, ced wrote: > It seems this is just enough: > > <svg version="1.0" xmlns="http://www.w3.org/2000/svg" width="24" height="24"/> > > I'm wondering if it really need to be in a file. Maybe it could be stored in > common as pixbuf. Done. https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:64: cell = args[1] On 2022/02/16 14:07:18, ced wrote: > Maybe the wrapper could use the signature: (self, cell, *args, *kwargs) Done. https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:66: cell.set_property('text', ' ') On 2022/02/16 14:07:19, ced wrote: > Is it enough when the content once realized is bigger than 1 char? Not it isn't but I don't think it really matters as the clipping would happen anyway if the value for one of the record displayed later was longer than the current value. So we just have to choose a "good" value. https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:69: _, width, height = Gtk.IconSize.lookup(Gtk.IconSize.MENU) On 2022/02/16 14:07:18, ced wrote: > Maybe the width and height could be defined also for this Cells. This would > avoid to duplicate which IconSize to use for each one. Done. https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:73: key = (id(self), width, height) On 2022/02/16 14:07:18, ced wrote: > Why should it be per id? Done. https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:80: On 2022/02/16 14:07:19, ced wrote: > not needed. Done.
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:66: cell.set_property('text', ' ') On 2022/02/16 18:29:21, nicoe wrote: > On 2022/02/16 14:07:19, ced wrote: > > Is it enough when the content once realized is bigger than 1 char? > > Not it isn't but I don't think it really matters as the clipping would happen > anyway if the value for one of the record displayed later was longer than the > current value. > > So we just have to choose a "good" value. Then I do not think it is a good solution.
Sign in to reply to this message.
Review's title does not follow the convention: '^(?P<repository>[A-Za-z_][\w\.-]+)(?P<version> [0-9.]+)?:' URL: https://codereview.tryton.org/370861002
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:66: cell.set_property('text', ' ') On 2022/02/16 18:36:48, ced wrote: > On 2022/02/16 18:29:21, nicoe wrote: > > On 2022/02/16 14:07:19, ced wrote: > > > Is it enough when the content once realized is bigger than 1 char? > > > > Not it isn't but I don't think it really matters as the clipping would happen > > anyway if the value for one of the record displayed later was longer than the > > current value. > > > > So we just have to choose a "good" value. > > Then I do not think it is a good solution. I think it is because this issue is somewhat there even for widget that are directly displayed : the size of those columns are defined by the biggest content for the X first records displayed.
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:66: cell.set_property('text', ' ') On 2022/02/16 21:31:28, nicoe wrote: > On 2022/02/16 18:36:48, ced wrote: > > On 2022/02/16 18:29:21, nicoe wrote: > > > On 2022/02/16 14:07:19, ced wrote: > > > > Is it enough when the content once realized is bigger than 1 char? > > > > > > Not it isn't but I don't think it really matters as the clipping would > happen > > > anyway if the value for one of the record displayed later was longer than > the > > > current value. > > > > > > So we just have to choose a "good" value. > > > > Then I do not think it is a good solution. > > I think it is because this issue is somewhat there even for widget that are > directly displayed : the size of those columns are defined by the biggest > content for the X first records displayed. For for displayed list, they are realized and so the real content is used.
Sign in to reply to this message.
Got this error when launching gtk client on linux with latest patch:
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/main.py",
line 258, in do_command_line
self.do_activate()
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/main.py",
line 255, in do_activate
self.get_preferences()
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/main.py",
line 532, in get_preferences
self.sig_win_menu(prefs=prefs)
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/main.py",
line 739, in sig_win_menu
screen = Screen(action['res_model'], mode=['tree'], view_ids=view_ids,
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/window/view_form/screen/screen.py",
line 140, in __init__
self.switch_view()
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/window/view_form/screen/screen.py",
line 555, in switch_view
self.load_view_to_load()
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/window/view_form/screen/screen.py",
line 580, in load_view_to_load
self.add_view_id(view_id, view_type)
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/window/view_form/screen/screen.py",
line 596, in add_view_id
return self.add_view(view)
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/window/view_form/screen/screen.py",
line 623, in add_view
view = View.parse(
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/window/view_form/view/__init__.py",
line 68, in parse
from .list import ViewTree
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/window/view_form/view/list.py",
line 24, in <module>
from .list_gtk.widget import (
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/gui/window/view_form/view/list_gtk/widget.py",
line 57, in <module>
EMPTY_IMG = data2pixbuf(EMPTY_SVG)
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/common/common.py",
line 1297, in data2pixbuf
return _data2pixbuf(data, width, height)
File
"/home/mrichez/Workspace/tryton/issues/issue_11214_volume_dimensions_package/tryton/tryton/common/common.py",
line 1289, in _data2pixbuf
loader.write(data)
TypeError: Item 0: Must be number, not str
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:66: cell.set_property('text', ' ') On 2022/02/16 23:13:03, ced wrote: > On 2022/02/16 21:31:28, nicoe wrote: > > On 2022/02/16 18:36:48, ced wrote: > > > On 2022/02/16 18:29:21, nicoe wrote: > > > > On 2022/02/16 14:07:19, ced wrote: > > > > > Is it enough when the content once realized is bigger than 1 char? > > > > > > > > Not it isn't but I don't think it really matters as the clipping would > > happen > > > > anyway if the value for one of the record displayed later was longer than > > the > > > > current value. > > > > > > > > So we just have to choose a "good" value. > > > > > > Then I do not think it is a good solution. > > > > I think it is because this issue is somewhat there even for widget that are > > directly displayed : the size of those columns are defined by the biggest > > content for the X first records displayed. > > For for displayed list, they are realized and so the real content is used. Yes but if for whatever reason the symbol for the 100th lines is longer than the one of the 1st line, since the data will be filled when fetched the clipping of the symbol will occur anyway because the symbol column won't be resized.
Sign in to reply to this message.
Review's title does not follow the convention: '^(?P<repository>[A-Za-z_][\w\.-]+)(?P<version> [0-9.]+)?:' URL: https://codereview.tryton.org/370861002
Sign in to reply to this message.
No more error when launching but symbols are still truncated :( https://pasteboard.co/9smbo4vUe6h0.png
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:66: cell.set_property('text', ' ') On 2022/02/17 09:17:43, nicoe wrote: > On 2022/02/16 23:13:03, ced wrote: > > On 2022/02/16 21:31:28, nicoe wrote: > > > On 2022/02/16 18:36:48, ced wrote: > > > > On 2022/02/16 18:29:21, nicoe wrote: > > > > > On 2022/02/16 14:07:19, ced wrote: > > > > > > Is it enough when the content once realized is bigger than 1 char? > > > > > > > > > > Not it isn't but I don't think it really matters as the clipping would > > > happen > > > > > anyway if the value for one of the record displayed later was longer > than > > > the > > > > > current value. > > > > > > > > > > So we just have to choose a "good" value. > > > > > > > > Then I do not think it is a good solution. > > > > > > I think it is because this issue is somewhat there even for widget that are > > > directly displayed : the size of those columns are defined by the biggest > > > content for the X first records displayed. > > > > For for displayed list, they are realized and so the real content is used. > > Yes but if for whatever reason the symbol for the 100th lines is longer than the > one of the 1st line, since the data will be filled when fetched the clipping of > the symbol will occur anyway because the symbol column won't be resized. Indeed the behavior is described in https://docs.gtk.org/gtk3/class.CellArea.html After reading the internal of Gtk.TreeView, Gtk.TreeViewColum etc. I think we could solve the problem by calling https://docs.gtk.org/gtk3/method.TreeViewColumn.queue_resize.html when the cell goes from not realized to realized. And we will no more need to fill non realized with blank. This is because TreeViewColumn.queue_resize calls _gtk_tree_view_column_cell_set_dirty which calls _gtk_tree_view_install_mark_rows_col_dirty which calls install_presize_handler which starts a thread for validate_rows which should call do_presize_handler and compute correct size.
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:66: cell.set_property('text', ' ') On 2022/02/17 10:08:51, ced wrote: > On 2022/02/17 09:17:43, nicoe wrote: > > On 2022/02/16 23:13:03, ced wrote: > > > On 2022/02/16 21:31:28, nicoe wrote: > > > > On 2022/02/16 18:36:48, ced wrote: > > > > > On 2022/02/16 18:29:21, nicoe wrote: > > > > > > On 2022/02/16 14:07:19, ced wrote: > > > > > > > Is it enough when the content once realized is bigger than 1 char? > > > > > > > > > > > > Not it isn't but I don't think it really matters as the clipping would > > > > happen > > > > > > anyway if the value for one of the record displayed later was longer > > than > > > > the > > > > > > current value. > > > > > > > > > > > > So we just have to choose a "good" value. > > > > > > > > > > Then I do not think it is a good solution. > > > > > > > > I think it is because this issue is somewhat there even for widget that > are > > > > directly displayed : the size of those columns are defined by the biggest > > > > content for the X first records displayed. > > > > > > For for displayed list, they are realized and so the real content is used. > > > > Yes but if for whatever reason the symbol for the 100th lines is longer than > the > > one of the 1st line, since the data will be filled when fetched the clipping > of > > the symbol will occur anyway because the symbol column won't be resized. > > Indeed the behavior is described in > https://docs.gtk.org/gtk3/class.CellArea.html > > After reading the internal of Gtk.TreeView, Gtk.TreeViewColum etc. > I think we could solve the problem by calling > https://docs.gtk.org/gtk3/method.TreeViewColumn.queue_resize.html when the cell > goes from not realized to realized. And we will no more need to fill non > realized with blank. > > This is because TreeViewColumn.queue_resize calls > _gtk_tree_view_column_cell_set_dirty which calls > _gtk_tree_view_install_mark_rows_col_dirty which calls install_presize_handler > which starts a thread for validate_rows which should call do_presize_handler and > compute correct size. There is already a queue_redraw called on the display. I guess it doesn't call the queue_resize (I haven't look at it yet).
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:66: cell.set_property('text', ' ') On 2022/02/17 10:33:36, nicoe wrote: > On 2022/02/17 10:08:51, ced wrote: > > On 2022/02/17 09:17:43, nicoe wrote: > > > On 2022/02/16 23:13:03, ced wrote: > > > > On 2022/02/16 21:31:28, nicoe wrote: > > > > > On 2022/02/16 18:36:48, ced wrote: > > > > > > On 2022/02/16 18:29:21, nicoe wrote: > > > > > > > On 2022/02/16 14:07:19, ced wrote: > > > > > > > > Is it enough when the content once realized is bigger than 1 char? > > > > > > > > > > > > > > Not it isn't but I don't think it really matters as the clipping > would > > > > > happen > > > > > > > anyway if the value for one of the record displayed later was longer > > > than > > > > > the > > > > > > > current value. > > > > > > > > > > > > > > So we just have to choose a "good" value. > > > > > > > > > > > > Then I do not think it is a good solution. > > > > > > > > > > I think it is because this issue is somewhat there even for widget that > > are > > > > > directly displayed : the size of those columns are defined by the > biggest > > > > > content for the X first records displayed. > > > > > > > > For for displayed list, they are realized and so the real content is used. > > > > > > Yes but if for whatever reason the symbol for the 100th lines is longer than > > the > > > one of the 1st line, since the data will be filled when fetched the clipping > > of > > > the symbol will occur anyway because the symbol column won't be resized. > > > > Indeed the behavior is described in > > https://docs.gtk.org/gtk3/class.CellArea.html > > > > After reading the internal of Gtk.TreeView, Gtk.TreeViewColum etc. > > I think we could solve the problem by calling > > https://docs.gtk.org/gtk3/method.TreeViewColumn.queue_resize.html when the > cell > > goes from not realized to realized. And we will no more need to fill non > > realized with blank. > > > > This is because TreeViewColumn.queue_resize calls > > _gtk_tree_view_column_cell_set_dirty which calls > > _gtk_tree_view_install_mark_rows_col_dirty which calls install_presize_handler > > which starts a thread for validate_rows which should call do_presize_handler > and > > compute correct size. > > There is already a queue_redraw called on the display. > I guess it doesn't call the queue_resize (I haven't look at it yet). ViewTree.display is not called when the TreeView become realized. I do not think the TreeView redraw will force the recomputation of the column size if this one has not changed for GTK. Indeed we need to mark the column cell as dirty once with change the content without triggering an event (from the model). But the important is in this decorator that must do something when the realized status change (and this is not linked to the display calls).
Sign in to reply to this message.
Review's title does not follow the convention: '^(?P<repository>[A-Za-z_][\w\.-]+)(?P<version> [0-9.]+)?:' URL: https://codereview.tryton.org/370861002
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:66: cell.set_property('text', ' ') On 2022/02/17 10:41:14, ced wrote: > On 2022/02/17 10:33:36, nicoe wrote: > > On 2022/02/17 10:08:51, ced wrote: > > > On 2022/02/17 09:17:43, nicoe wrote: > > > > On 2022/02/16 23:13:03, ced wrote: > > > > > On 2022/02/16 21:31:28, nicoe wrote: > > > > > > On 2022/02/16 18:36:48, ced wrote: > > > > > > > On 2022/02/16 18:29:21, nicoe wrote: > > > > > > > > On 2022/02/16 14:07:19, ced wrote: > > > > > > > > > Is it enough when the content once realized is bigger than 1 > char? > > > > > > > > > > > > > > > > Not it isn't but I don't think it really matters as the clipping > > would > > > > > > happen > > > > > > > > anyway if the value for one of the record displayed later was > longer > > > > than > > > > > > the > > > > > > > > current value. > > > > > > > > > > > > > > > > So we just have to choose a "good" value. > > > > > > > > > > > > > > Then I do not think it is a good solution. > > > > > > > > > > > > I think it is because this issue is somewhat there even for widget > that > > > are > > > > > > directly displayed : the size of those columns are defined by the > > biggest > > > > > > content for the X first records displayed. > > > > > > > > > > For for displayed list, they are realized and so the real content is > used. > > > > > > > > Yes but if for whatever reason the symbol for the 100th lines is longer > than > > > the > > > > one of the 1st line, since the data will be filled when fetched the > clipping > > > of > > > > the symbol will occur anyway because the symbol column won't be resized. > > > > > > Indeed the behavior is described in > > > https://docs.gtk.org/gtk3/class.CellArea.html > > > > > > After reading the internal of Gtk.TreeView, Gtk.TreeViewColum etc. > > > I think we could solve the problem by calling > > > https://docs.gtk.org/gtk3/method.TreeViewColumn.queue_resize.html when the > > cell > > > goes from not realized to realized. And we will no more need to fill non > > > realized with blank. > > > > > > This is because TreeViewColumn.queue_resize calls > > > _gtk_tree_view_column_cell_set_dirty which calls > > > _gtk_tree_view_install_mark_rows_col_dirty which calls > install_presize_handler > > > which starts a thread for validate_rows which should call do_presize_handler > > and > > > compute correct size. > > > > There is already a queue_redraw called on the display. > > I guess it doesn't call the queue_resize (I haven't look at it yet). > > ViewTree.display is not called when the TreeView become realized. > I do not think the TreeView redraw will force the recomputation of the column > size if this one has not changed for GTK. > Indeed we need to mark the column cell as dirty once with change the content > without triggering an event (from the model). But the important is in this > decorator that must do something when the realized status change (and this is > not linked to the display calls). What do you mean by "mark the column durity once with change the content without triggering an event (from the model)" ?
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/390071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:66: cell.set_property('text', ' ') On 2022/03/14 18:15:11, nicoe wrote: > On 2022/02/17 10:41:14, ced wrote: > > On 2022/02/17 10:33:36, nicoe wrote: > > > On 2022/02/17 10:08:51, ced wrote: > > > > On 2022/02/17 09:17:43, nicoe wrote: > > > > > On 2022/02/16 23:13:03, ced wrote: > > > > > > On 2022/02/16 21:31:28, nicoe wrote: > > > > > > > On 2022/02/16 18:36:48, ced wrote: > > > > > > > > On 2022/02/16 18:29:21, nicoe wrote: > > > > > > > > > On 2022/02/16 14:07:19, ced wrote: > > > > > > > > > > Is it enough when the content once realized is bigger than 1 > > char? > > > > > > > > > > > > > > > > > > Not it isn't but I don't think it really matters as the clipping > > > would > > > > > > > happen > > > > > > > > > anyway if the value for one of the record displayed later was > > longer > > > > > than > > > > > > > the > > > > > > > > > current value. > > > > > > > > > > > > > > > > > > So we just have to choose a "good" value. > > > > > > > > > > > > > > > > Then I do not think it is a good solution. > > > > > > > > > > > > > > I think it is because this issue is somewhat there even for widget > > that > > > > are > > > > > > > directly displayed : the size of those columns are defined by the > > > biggest > > > > > > > content for the X first records displayed. > > > > > > > > > > > > For for displayed list, they are realized and so the real content is > > used. > > > > > > > > > > Yes but if for whatever reason the symbol for the 100th lines is longer > > than > > > > the > > > > > one of the 1st line, since the data will be filled when fetched the > > clipping > > > > of > > > > > the symbol will occur anyway because the symbol column won't be resized. > > > > > > > > Indeed the behavior is described in > > > > https://docs.gtk.org/gtk3/class.CellArea.html > > > > > > > > After reading the internal of Gtk.TreeView, Gtk.TreeViewColum etc. > > > > I think we could solve the problem by calling > > > > https://docs.gtk.org/gtk3/method.TreeViewColumn.queue_resize.html when the > > > cell > > > > goes from not realized to realized. And we will no more need to fill non > > > > realized with blank. > > > > > > > > This is because TreeViewColumn.queue_resize calls > > > > _gtk_tree_view_column_cell_set_dirty which calls > > > > _gtk_tree_view_install_mark_rows_col_dirty which calls > > install_presize_handler > > > > which starts a thread for validate_rows which should call > do_presize_handler > > > and > > > > compute correct size. > > > > > > There is already a queue_redraw called on the display. > > > I guess it doesn't call the queue_resize (I haven't look at it yet). > > > > ViewTree.display is not called when the TreeView become realized. > > I do not think the TreeView redraw will force the recomputation of the column > > size if this one has not changed for GTK. > > Indeed we need to mark the column cell as dirty once with change the content > > without triggering an event (from the model). But the important is in this > > decorator that must do something when the realized status change (and this is > > not linked to the display calls). > > What do you mean by "mark the column durity once with change the content without > triggering an event (from the model)" ? Oops some typo, I mean: "mark the column as dirty once we change the content with triggering an event". I mean that the queue_redraw in display is not enough because the realized property can change without having a call to display. So we need that this decorator makes the call to queue_resize when the treeview changed from not realized to realized (but only once).
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/409071002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/409071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:52: def realized(func): I'm wondering if it will not be cleaner to define a variable here: has_been_realized = False https://codereview.tryton.org/370861002/diff/409071002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:54: def wrapper(self, column, cell, *args, **kwargs): no more needed
Sign in to reply to this message.
https://codereview.tryton.org/370861002/diff/380721002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:62: undefined name 'column' https://codereview.tryton.org/370861002/diff/380721002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:62: undefined name 'cell' URL: https://codereview.tryton.org/370861002
Sign in to reply to this message.
The title must be updated. https://codereview.tryton.org/370861002/diff/380721002/tryton/tryton/gui/wind... File tryton/tryton/gui/window/view_form/view/list_gtk/widget.py (right): https://codereview.tryton.org/370861002/diff/380721002/tryton/tryton/gui/wind... tryton/tryton/gui/window/view_form/view/list_gtk/widget.py:62: return func(self, column, cell, *args, **kwargs) missing removal
Sign in to reply to this message.
checks OK URL: https://codereview.tryton.org/370861002
Sign in to reply to this message.
For me the title is still not corresponding to the change.
Sign in to reply to this message.
On 2022/04/06 07:56:42, ced wrote: > For me the title is still not corresponding to the change. I updated the title (don't know if reviewers are warned about that).
Sign in to reply to this message.
New changeset d415b76ceb89 by Nicolas Évrard in branch 'default': Request a column size recomputation on treeview realization https://hg.tryton.org/tryton/rev/d415b76ceb89
Sign in to reply to this message.
New changeset d3ab8e59c06f by Nicolas Évrard in branch 'default': Request a column size recomputation on treeview realization https://hg.tryton.org/tryton-env/rev/d3ab8e59c06f
Sign in to reply to this message.
New changeset 1f1a5356caff by Nicolas Évrard in branch '6.2': Request a column size recomputation on treeview realization https://hg.tryton.org/tryton/rev/1f1a5356caff New changeset e5fbd9092453 by Nicolas Évrard in branch '6.0': Request a column size recomputation on treeview realization https://hg.tryton.org/tryton/rev/e5fbd9092453 New changeset 48a7129d20ad by Nicolas Évrard in branch '5.0': Request a column size recomputation on treeview realization https://hg.tryton.org/tryton/rev/48a7129d20ad
Sign in to reply to this message.
|
