Ticket #3716 (closed defect: fixed)
Options: Disabled status not toggled immediately
Reported by: | egmont | Owned by: | andrew_b |
---|---|---|---|
Priority: | minor | Milestone: | 4.8.19 |
Component: | mc-core | Version: | master |
Keywords: | Cc: | ||
Blocked By: | Blocking: | ||
Branch state: | merged | Votes for changeset: | committed-master |
Description
Go to Options -> Configuration. Toggle "Single press" (either with keyboard or with mouse).
Expected: "Timeout" immediately becomes enabled/disabled accordingly.
Actual: "Timeout" becomes properly enabled/disabled only after the focus leaves and then re-enters "Single press".
The same goes for other potentially disabled fields as well, i.e. Layout -> Equal split, and Virtual FS -> Always use ftp proxy.
Attachments
Change History
comment:3 in reply to: ↑ 1 Changed 8 years ago by andrew_b
Replying to mooffie:
@Andrew: I see that focusing a widget sends the dialog a MSG_NOTIFY. Is this really needed?
Really needed what? Send some message to dialog? Definitely yes. Send a MSG_NOTIFY? Let's discuss.
When I wrote c37f9b770a7af246d59e01e0746a83132a1a16ab I had a choice between MSG_NOTIFY and MSG_FOCUS/MSG_UNFOCUS. MSG_(UN)FOCUS have the only sense: these messages are sent to change widget's focus state. In this logic, if you send MSG_(UN)FOCUS to a dialog, it should change it's focus state. Currently dialog can't be (un)focused because it's unused and not implemented, but it is not a reason to send MSG_(UN)FOCUS to a dialog. Therefore I applied MSG_NOTIFY to notify widget's owner about changing focus of a widget.
I can't agree with usage of MSG_KEY instead of MSG_FOCUS as parameter of MSG_NOTIFY because key is just one of two cases of (un)focusing. The second key is mouse. Perhaps we should create a special message (say, MSG_CHANGE_FOCUS).
comment:4 follow-up: ↓ 5 Changed 8 years ago by mooffie
Ideally I should respond after studying the recent API changes, but since I may be away from the computer for a day or two I'll give, for the meantime, my quick uninformed reading of the situation (so don't let it raise your blood pressure!):
- MSG_NOTIFY was meant to be like the extremely useful on_change from other GUIs. It was meant to inform the dialog (or related widgets) that a widget has changed its value. Or possibly that some useful event peculiar to it has occurred ("I've finished loading the file!").
- Then, if I understand correctly, because you wanted to inform the dialog of a focus change, you split MSG_NOTIFY into two. The old meaning (that of on_change) has parm==MSG_KEY (even when caused by the mouse) and the new meaning has parm==MSG_FOCUS.
- In other words, you carried MSG_NOTIFY's meaning farther than I initially expected. You use it to notify the dialog of changes to w->state. Judging by the result, this makes the code less pretty. To me this looks like the sad situation we had with MSG_ACTION, which used to serve two purposes.
Perhaps we should create a special message (say, MSG_CHANGE_FOCUS).
Certainly! You mean, instead of MSG_NOTIFY[MSG_FOCUS], right? No need to bastardize MSG_NOTIFY. Creating a new message has a huge benefit: we can immediately see if we use it at all, where we use it, and why the message needs to exist.
MSG_(UN)FOCUS have the only sense: these messages are sent to change widget's focus state.
I didn't know that's the new meaning of MSG_(UN)FOCUS.
Are you sure your wording is exact? I thought its meaning now would be to tell the widget: "You've just (lost)received the focus!". The widget, I thought, has already got its state (WST_FOCUSED) changed by this point. It'd be followed by MSG_DRAW. Again: I'll need to study the code.
In the past I suggested to break MSG_FOCUS into MSG_QUERY_FOCUS ("tell me, are you able to receive the focus?", to which it should answer yes/no) and MSG_GOT_FOCUS ("You've just received the focus!", to which it won't need to answer). (This MSG_GOT_FOCUS could be renamed "MSG_FOCUS", as before.) Similarly with MSG_UNFOCUS.
I can't agree with usage of MSG_KEY instead of MSG_FOCUS as parameter of MSG_NOTIFY because key is just one of two cases of (un)focusing. The second key is mouse.
My opinion is that MSG_NOTIFY should have one purpose, and therefore should need no parameter (at least not one that changes the meaning).
My patches weren't meant to be committed. They were just a way to quickly show what caused the bug.
I can't agree with usage of MSG_KEY [...] because key is just one of two cases of (un)focusing. The second key is mouse.
I hope this is a typo. Why should we care how a widget got/lost focus? Or whether its data change (MSG_NOTIFY) was caused by mouse or keyboard?
comment:5 in reply to: ↑ 4 Changed 8 years ago by andrew_b
Replying to mooffie:
Ideally I should respond after studying the recent API changes
You can start from 5ac1c5a3e0131f49cd78bad69a5bb0068b9f02f2.
I can't agree with usage of MSG_KEY [...] because key is just one of two cases of (un)focusing. The second key is mouse.
I hope this is a typo.
Yes, this is a typo. s/second key/second case
Why should we care how a widget got/lost focus? Or whether its data change (MSG_NOTIFY) was caused by mouse or keyboard?
Exactly. MSG_NOTIGY is the general message, it doesn't depend upon event source. The name MSG_KEY means the keyboard as source.
comment:6 Changed 8 years ago by andrew_b
- Owner set to andrew_b
- Status changed from new to accepted
- Votes for changeset set to andrew_b
- Branch state changed from no branch to on review
- Milestone changed from Future Releases to 4.8.19
Branch: 3716_checkbox_response
changeset:b4d5ed514d1d65964ced22e13e2894c54cc2865b
comment:8 Changed 8 years ago by andrew_b
- Status changed from accepted to testing
- Votes for changeset changed from andrew_b to committed-master
- Resolution set to fixed
- Branch state changed from approved to merged
Merged to master: [84433f43c24568c28f38e42638bb3cb68e1ca5b7].
comment:10 follow-up: ↓ 12 Changed 8 years ago by mooffie
Great. I'm relieved to see the new MSG_CHANGED_FOCUS.
What I still don't like is the parm == MSG_KEY / MSG_WHATEVER accompanying MSG_NOTIFY, and here's why:
I see MSG_NOTIFY as a "high level" event. Or, using other words, "application level". Or, if you will, "semantic event". Most other events are low-level: they have to do with the implementation of the widget system.
Users of MSG_NOTIFY are supposed to be interested in semantic stuff like "value has changed", "data is ready", etc. I don't see what low-level stuff like MSG_KEY (or, previously, MSG_FOCUS) has to do with this. Seems like mixing oranges with East Siberian bears. MSG_NOTIFY was supposed, I believed, to free users from having to work with exactly this sort of low level mechanics; to make them able to concentrate on just what they want.
comment:11 Changed 8 years ago by mooffie
Why is it necessary for these boxes.c dialogs to check that their radios/checboxes were modified by the keyboard and not, say, by the mouse?
comment:12 in reply to: ↑ 10 Changed 8 years ago by andrew_b
Replying to mooffie:
Great. I'm relieved to see the new MSG_CHANGED_FOCUS.
What I still don't like is the parm == MSG_KEY / MSG_WHATEVER accompanying MSG_NOTIFY:
I applied patch as is to fix bug in short time.
In general, I agree. WCheck and WRadio send MSG_NOTIFY in the only one case: when state is changed. Therefore we can omit parm == MSG_KEY / MSG_WHATEVER. I'll make it in the cleanup branch.
This is because of a newfangled change. The checkbox does:
but the dialog checks:
It checks for MSG_FOCUS, not MSG_KEY. (There are more places like this.)
@Andrew: I see that focusing a widget sends the dialog a MSG_NOTIFY. Is this really needed?