Disable labels with the controls they are associated with#11812
Conversation
…ls, disable the associated label when the control is disabled.
feerrenrut
left a comment
There was a problem hiding this comment.
Have you considered using events for this? I think it might be simpler. I had a quick look, but didn't find an event for enable/disable, if there is not you could use a custom event.
See https://docs.wxpython.org/events_overview.html#events-and-event-handling
The code has totally been reworked according to @feerenrut's suggestion.
|
@feerrenrut thanks for this review and the much better code suggestion. My initial code was clearly not correct regarding the missed new method. On a functional point of view, I have just added a try/catch in the _onDestroy handler called when the label is destroyed:
|
Yes, it seems likely. The destroy event being used like this seems to be making things more complicated. It's a shame there is no 'isDestroyed' method on a class LabelEnableChangedListener(wx.StaticText):
isDestroyed = False
isListening = False
def _onDestroy(self, evt: wx.WindowDestroyEvent):
self.isDestroyed = True
def listenForEnableChanged(self, _ctrl: wx.Window):
self.Bind(wx.EVT_WINDOW_DESTROY, self._onDestroy)
_ctrl.Bind(LabeledControlHelper.EVT_ENABLE_CHANGED, self._onEnableChanged)
self.isListening = True
def _onEnableChanged(self, evt: wx.Event):
if self.isListening and not self.isDestroyed:
self.Enable(evt.isEnabled)
class WxCtrlWithEnableEvnt(wxCtrlClass):
def Enable(self, enable=True):
evt = LabeledControlHelper.EnableChanged(isEnabled=enable)
wx.PostEvent(self, evt)
super().Enable(enable)
def Disable(self):
evt = LabeledControlHelper.EnableChanged(isEnabled=False)
wx.PostEvent(self, evt)
super().Disable()
self._label = LabelEnableChangedListener(parent, label=labelText)
self._ctrl = WxCtrlWithEnableEvnt(parent, **kwargs)
self._label.listenForEnableChanged(self._ctrl)
self._sizer = associateElements(self._label, self._ctrl) |
|
Not sure to understand. Are you fixing something else in your new proposal (in #11812 (comment))? Or is it just to organize the code better, defining a subclass of wx.Label rather than binding sparse functions with events? No problem to modify my code following your last suggestion. However, I just want to be sure to fully understand what I am committing! |
|
Yes, this is just an alternative approach. I think it's more tidy, it means that the order of destruction doesn't matter anymore. The core of the problem is understanding that the lifetime of the wx (c++ object) does not necessarily match the lifetime of the python object. I figure this is a neater solution because it doesn't tie the two python objects together in any additional way, like my first suggestion with the introduction of the label listening to the destroy event of the control. This new approach ensure that the label is only watching the control, but will not attempt to modify itself if the C++ object has been destroyed already. |
Credit: Reef Turner
|
Thanks for this clear and detailed explanation. |
Link to issue number:
Fixes #11809
Summary of the issue:
When some UI controls such as combo-box or edit field are disabled (greyed out), the associated label remains enabled. For low vision people this makes it harder to detect that the option has been disabled. Keeping the label enabled also lets the accelerator key active; thus pressing this acccelerator will make the focus jump to the next focusable control that is not disabled. This is an unexpected behaviour.
Description of how this pull request fixes the issue:
guiHelper.LabeledControlHelper.control now returns an object whose class inherit both from BaseLabeledControl and the wxCtrlClass class that was passed to the constructor.
This object behaves the same way as the wxCtrlClass except for Enable and Disable methods.
The new BaseLabeledControl class contains the code to manage enabling and disabling the (main) control as well as the associated label. See the example in the BaseLabeledControl doc string for usage.
Thus any labeled controls that was created by guiHelper.LabeledControlHelper in the UI disables or enables its associated label along with itself.
Testing performed:
In the Braille settings panel:
Notes
I am not an expert in advanced concepts ofOOP Python object programming. Thus, any comment to refine my code is welcome. Especially:
Moreover the group name in the advanced setting panel are not greyed out with the rest of the panel. I do not know how to grey it out and, reading the Wx Python documentation, it seems this is an intended behaviour. So I did not dig further.
Known issues with pull request:
None
Change log entry:
Not needed.
Cc @feerenrut for review.