search

enthought / traitsui

TraitsUI: Traits-capable windowing framework
http://docs.enthought.com/traitsui
Other
297 stars 96 forks source link

Creation and toolkit agnostic configuration of an editor belong to editor factory, not UI panel #984

Open kitchoi opened 4 years ago

kitchoi commented 4 years ago

The following code in traitsui.qt4.ui_panel is identical to the one in traitsui.wx.ui_panel: https://github.com/enthought/traitsui/blob/029443bf018f136e68fefe802b522e5365f86b5b/traitsui/qt4/ui_panel.py#L851-L867

This block contains toolkit agnostic code and can easily be moved to the toolkit agnostic editor factory. This refactoring would make it easier to test the logic without a GUI toolkit. It is also logically reasonable because the factory should be responsible for instantiating an editor as far as it is allowed within the toolkit agnostic boundary.

The original getattr(editor_factory, item.style + "_editor") is an implementation detail that belongs to the editor factory as well.

This is what I think the block will eventually look like (more or less):

editor = editor_factory.create_editor(ui, object, name, item, item_panel)
corranwebster commented 4 years ago

This whole file needs a major clean-up/re-write.

kitchoi commented 4 years ago

The if item.format_func is not None has since been moved out (see https://github.com/enthought/traitsui/pull/985). But the following two lines should be moved inside the factory: https://github.com/enthought/traitsui/blob/6fccc3966e89a8082aace9623b84a80a74c46f3e/traitsui/qt4/ui_panel.py#L865-L868 https://github.com/enthought/traitsui/blob/6fccc3966e89a8082aace9623b84a80a74c46f3e/traitsui/wx/ui_panel.py#L902-L906

This will make fixing https://github.com/enthought/traitsui/issues/901 easier. I think a lot of these duck typings are caused by the ~fact~ error that the factory action NOT being carried out by the factory itself.