Merge vnotebook into master to add VNotebook widget

[sbordeyne]

PR Details:

• Widget name: VNotebook
• Author: @RedFantom

Description

A Notebook with vertical tabs

Checklist

• Widget in a separate file in the appropriate folder
• Widget functions properly on both Windows and Linux
• Widget code includes docstrings with parameter descriptions
• Included an example file in /examples
• Widget is covered by unitttests in /tests
• Widget includes required assets files
• Reference to widget in AUTHORS.md
• Entry in sphinx documentation

[j4321]

I think the error in the python 3.4 test with Appveyor is because ttkthemes is used in the example_vnotebook.py and probably the older version of pip or setuptools does not recognize requirements properly so we should add it explicitly in .appveyor.yml. However I am reluctant to add ttkthemes as a dependency just for the sake of one example, we could use the 'clam' theme instead.

[j4321]

The widget does not seems fully finished, there is the callback method which is documented but not implemented and the add() and insert() method behavior should be improved for already existing tabs. I will help addressing my comments.

[j4321]

I have noticed a weird behavior with the add() method: When adding a widget which is already inside the notebook, a new tab is created, but the old one is not deleted and they are both activated simultaneously. I think we should add code to check whether the widget is already in the notebook and just update its properties with kwargs like the ttk.Notebook does.

[sbordeyne]

just fixed this

check my latest commit

[j4321]

The callback option does not seem to have been implemented yet, is that so?

[j4321]

By the way, wouldn't it be more consistent with the ttk.Notebook widget to use a virtual event, e.g. <<VNotebookTabChanged>> instead of a callback?

[sbordeyne]

I agree. A virtual event should be the way to go as it's more consistent with the rest of tkinter and ttk. It's also much easier to implement, and easier to document. Now, what data should the event argument hold in that case?

[j4321]

When I make tkinter print the <<NotebookTabChanged>> event, I get <VirtualEvent event x=0 y=0> so I think we don't have to pass any specific argument to the event, the new tab id and everything else can be recovered from the notebook's methods.

[sbordeyne]

I removed the 'callback' argument from the documentation, there didn't seem to be any code to remove with that change. I'll let you handle the event generation since you seem to be doing that.

[j4321]

When using .configure() I get the error

Traceback (most recent call last):
  File "/media/DATA/Divers/ttkwidgets/tests/test_vnotebook.py", line 77, in test_configure
    notebook.config(padding=6)
  File "/media/DATA/Divers/ttkwidgets/ttkwidgets/frames/vnotebook.py", line 264, in configure
    setattr(self, attr, kwargs.pop(option, getattr(self, attr)))
AttributeError: 'VNotebook' object has no attribute '_takefocus'

I think we need to had the line
self._takefocus = kwargs.pop("takefocus", False)
in __init__() to fix that. And the one for callback is missing as well but the option is not implemented yet.

Moreover, there is a self missing at l. 265: return ttk.Frame.configure(self, **kwargs)

Finally, if all arguments are in self.options, then kwargs is emptied and therefore the dict of all the frame options is returned which is a bit weird.

[sbordeyne]

for the self missing, shouldn't we just use super() instead of calling ttk.Frame directly ? It's much cleaner after all.

[j4321]

Yes we can use super() though I have not taken the habit to use it yet, but this does not solve the issue of the behavior of the function. I don't think that many people use .configure("<key>") since the output is not really usable in python (e.g. ('width', 'width', 'Width', 0, 0)). So maybe we can return nothing and just use the method for its main purpose which is to change the options of the widget.

[sbordeyne]

I removed the call to ttk.Frame, in favor of a super() call. We could just return whatever ttk.Frame.configure returns, that way it stays consistent with the rest of the tkinter framework.

[j4321]

I think we can add some more tests, like for the configure() method. I can write some.