Akuli / teek

Simpler alternative to tkinter.
MIT License
31 stars 5 forks source link

Treeview widget #12

Open codetent opened 5 years ago

codetent commented 5 years ago

Implementation of treeview widget (#4)

Akuli commented 5 years ago

This is nice :D I read through the code quickly, and it mostly looks good. I need to read and review this in more detail later.

Why do you need error handling in FallbackConfigDict? Does that give errors when a TreeviewColumn or TreeviewRow hasn't been added to a Treeview yet?

codetent commented 5 years ago

The main idea behind the FallbackConfigDict is that when a tcl command cannot be executed, a temporary value should be set or read which is synchronized afterwards.

I added this functionality because TreeviewRow and TreeviewColumn are useless before they are added to a Treeview (limitation of tk). If the object is not added, the _treeview attribute is not set of the class and therefore an exception is thrown which leads to accessing the fallback values.

I caught the generic expression for keeping the FallbackConfigDict usable for other purposes where another exception (not AttributeError) is raised.

Akuli commented 5 years ago

I added this functionality because TreeviewRow and TreeviewColumn are useless before they are added to a Treeview

There is a similar situation with the Canvas widget, and I solved it by just not allowing the users to create canvas items before they create the canvas, and it works well. I'm not perfectly happy with the canvas code. I think it would be better to not create an Item subclass like type('Item', (CanvasItem,), {'canvas': self}) for each Canvas instance, and instead just pass the canvas as an argument whenever a CanvasItem object is created.

There is also a similar situation with the Menu widget, but it's solved differenly there, because menu items are used like this

window.config['menu'] = teek.Menu([
    teek.MenuItem("File", [
        teek.MenuItem("New", hello),
        teek.MenuItem("Open", hello),
        teek.MenuItem("Save", hello),
        teek.MenuItem("Quit", hello),
    ]),
    teek.MenuItem("Edit", [
        teek.MenuItem("Cut", hello),
        teek.MenuItem("Copy", hello),
        teek.MenuItem("Paste", hello),
    ]),
])

If the menu would need to be created first, then it would look like this

file_menu = teek.Menu()
file_menu.add_item("New", hello)
file_menu.add_item("Open", hello)
file_menu.add_item("Save", hello)
file_menu.add_item("Quit", hello)

edit_menu = teek.Menu()
edit_menu.add_item("Cut", hello)
edit_menu.add_item("Copy", hello)
edit_menu.add_item("Paste", hello)

window.config['menu'] = teek.Menu()
window.config['menu'].add_submenu(file_menu)
window.config['menu'].add_submenu(edit_menu)

which is much more annoying. This doesn't happen with canvas items, because canvas items can't be put inside other canvas items, like menus can be added to other menus as submenus.

codetent commented 5 years ago

I thought about implementing this like the menu widget, but I was not sure if this is really the best way for the user.

I think I will change this behaviour by removing the FallbackDict and raise an exception if the user tries to set options of a row or column which is not added to a Treeview.

codetent commented 5 years ago

The last changes replace the FallbackDict with a dictionary which allows multiple handler methods. Additionally, I simplified the code and added more tests.

There was also a problem with the tests using the newest svglib library. I created an issue for that problem!

codetent commented 5 years ago

Finally, I added the changes from the review! Now, nesting of rows is also possible and headings can be configured independently from columns.

codetent commented 5 years ago

The CI tests on versions 3.4 and nightly fail now, because lxml does not support python 3.4 anymore and flake8 has a problem with the latest nightly build.

These problems are fixed with #14.