Promote LEDEditor to a first-class editor #779
Conversation
It now has both Qt and Wx backends, so no reason not to.
|
I notice that #778 removes the documentation for |
Rats. No it doesn't. It just moves it. Sorry for the noise. |
|
In the demo, the fields are marked "left aligned", "right aligned", etc., but I don't see any difference in the alignments. Should I?
(EDIT: this is with Python 3.8.2 / PyQt5 5.14.2 / macOS 10.14.6) |
|
Ah, I ran the demo before looking at the source, which says that alignment is wx-only. |
| # is also available online at http://www.enthought.com/licenses/BSD.txt | ||
| # Thanks for using Enthought open source! | ||
|
|
||
| """ A Traits UI editor that wraps a LED-style integer display. |
| # editor names for factory to find | ||
| ReadonlyEditor = LEDEditor | ||
| SimpleEditor = LEDEditor | ||
| CustomEditor = LEDEditor |
There was a problem hiding this comment.
Do we actually need to populate all the three styles? If the custom/readonly editors are not implemented, then in the future we will be certain that they are never used because attempts to set style='custom' or style='readonly' will be met with an error (I think). If we populate them, it will add constraint to future extensions when we do want the custom / readonly style to behave completely differently.
e.g. The TableEditor does not have a custom editor. ButtonEditor does not have a readonly editor. They don't seem to be missed.
| if ETSConfig.toolkit == 'wx': | ||
| from traitsui.wx.extra.led_editor import LEDEditor | ||
| else: | ||
| from traitsui.qt4.extra.led_editor import LEDEditor |
There was a problem hiding this comment.
Shall we keep traitsui.qt4.extra.led_editor and traitsui.wx.extra.led_editor as aliases? It costs almost nothing to maintain aliases. Otherwise this would be a breaking change.
| .. automodule:: traitsui.editors.led_editor | ||
| :members: | ||
| :undoc-members: | ||
| :show-inheritance: |
There was a problem hiding this comment.
The API documentation is now being generated automatically. So this file and traitsui.editors.rst are not needed any more.
| @@ -59,18 +53,12 @@ def update_editor(self): | |||
| """ | |||
| self.control.SetValue(self.str_value) | |||
|
|
|||
| def _alignment_changed(self): | |||
| if self.control is not None: | |||
| self.control.SetAlignment(LEDStyles[self.alignment]) | |||
There was a problem hiding this comment.
Could we make such a change in a separate PR such that this PR is more focused about exposing the existing editor as first-class / built-in / whatever editor?
Furthermore I expect there being very limited use case to mutate the alignment after the editor is created - we can wait for this feature request to arise. The fewer mutables there are, the simpler and more maintainable the code is.
| def init(self, parent): | ||
| """ Finishes initializing the editor by creating the underlying toolkit | ||
| widget. | ||
| """ | ||
| self.control = LEDNumberCtrl(parent, -1) | ||
| self.control.SetAlignment(LEDStyles[self.factory.alignment]) | ||
| self.control.SetAlignment(LEDStyles[self.alignment]) |
There was a problem hiding this comment.
Same here, I think we can defer making alignment mutable in a separate PR / issue and restore the original code which uses the factory value.
(I bet one probably needs to sync_value method like this
traitsui/traitsui/qt4/list_str_editor.py
Line 114 in 7841ca7
There was a problem hiding this comment.
(I bet one probably needs to sync_value method like this...
Please ignore this half of the comment. The sync_value is used for synchronizing a trait on the editor with a trait on the object being edited. But the first half of the comment about defer making alignment mutable still stands.
It now has both Qt and Wx backends, so no reason not to.