Thread: [UFO-devel] Accelerators
Status: Beta
Brought to you by:
schmidtjf
From: Andreas B. <b_...@gm...> - 2005-06-07 22:12:39
|
Hi there is a pretty huge problem with the accelerator implementation: 1. create a button "button" and assign some action to it 2. button->setText("&Foobar"); 3. button->setVisible(false); 4. press ALT+F -> the action is executed, although the button is not visible anymore. This could be fixed e.g. by using a more object oriented design for the accelerators: simply use the key events in the corresponding widgets, instead of the input map of the root widget. CU Andi |
From: Johannes S. <sch...@us...> - 2005-06-08 07:30:47
|
Am Mittwoch 08 Juni 2005 00:12 schrieb Andreas Beckermann: > Hi > there is a pretty huge problem with the accelerator implementation: > 1. create a button "button" and assign some action to it > 2. button->setText("&Foobar"); > 3. button->setVisible(false); > 4. press ALT+F > -> the action is executed, although the button is not visible anymore. > > This could be fixed e.g. by using a more object oriented design for the > accelerators: simply use the key events in the corresponding widgets, > instead of the input map of the root widget. Hmm, I have to admit that I didn't understand your suggestion. Only the focused widget gets key events, but the accelerators/shortcuts should work no matter which widget is focused. The whole accelerator code needs to be improved anyhow. I thought of a global shortcut map (possibly within UDisplay). If a key event triggers a short cut, either it sends a shortcut event to all registered widget (which may then decide whether they want to process it), or an associated action slot is executed (similar to the current solution). As quick fix for your problem, the UButton::keybindingSlot method should be modified to check for visibility, isEnabled, bla. Regards, Johannes |
From: Andreas B. <b_...@gm...> - 2005-06-08 14:35:31
|
On Wednesday 08 June 2005 09:30, Johannes Schmidt wrote: > As quick fix for your problem, the UButton::keybindingSlot method should be > modified to check for visibility, isEnabled, bla. Just a short notification: By implementing something like this (you need to check isVisible()/isEnabled() for all parent widgets, too) the primary problem can be fixed. However at least one related problems remains: When there are two different buttons with the same accelerator which both can be visible (but only one at a time), then one accelerator does not work at all (although it is underlined correctly and therefore recognized by the user as "existing"). This is because libufo stores accelerators globally, but can store at most one slot per accel. Example (taken from boson): * Welcome widget with button "S&tart New Game" * NewGame widget with button "S&tart" The "Welcome" widget is visible initially. From there the user can (e.g.) enter the "NewGame" widget which causes the welcome widget to be hidden and the NewGame widget to be shown instead. This example can be made more complex by including the "Start &Editor" button which shows the "StartEditor" widget that also provides a "S&tart" button. And of course both widgets have a "&Cancel" button... CU Andi |
From: Andreas B. <b_...@gm...> - 2005-06-08 11:32:36
|
On Wednesday 08 June 2005 09:30, Johannes Schmidt wrote: > Am Mittwoch 08 Juni 2005 00:12 schrieb Andreas Beckermann: > > Hi > > there is a pretty huge problem with the accelerator implementation: > > 1. create a button "button" and assign some action to it > > 2. button->setText("&Foobar"); > > 3. button->setVisible(false); > > 4. press ALT+F > > -> the action is executed, although the button is not visible anymore. > > > > This could be fixed e.g. by using a more object oriented design for the > > accelerators: simply use the key events in the corresponding widgets, > > instead of the input map of the root widget. > > Hmm, I have to admit that I didn't understand your suggestion. > > Only the focused widget gets key events, but the accelerators/shortcuts > should work no matter which widget is focused. Hmm, yeah I did not think about that. What about this: when a KeyPressed event occurs: 1. Send an Accel event down the whole hierarchy (starting at root) 2. A widget that receives an Accel event checks whether it 3. If the Accel event is not consumed: send a KeyPressed event just like before 4. If the Accel event is consumed: stop (do not send KeyPressed event) And while we're on it this could be slightly extended similar to the way it works in Qt: before sending an Accel event, send an AccelOverride event. If that one is consumed, skip the Accel event and send a KeyPressed event directly. Otherwise continue as described above. -> This way it could be avoided that Accels take the key press instead of it being delivered to the correct widget (e.g. a lineedit which has the focus). An alternative to this procedure would be to continue using a global Accelerator cellection, but also storing the associated widget, so that widget->isVisible() and widget->isEnabled() can be checked. Doing that in keybindingSlot() is indeed a temporary solution only, as this would have to be done for every class that may provide accelerators. With this global solution (which is what is used by Qt, btw) the previously described event algorithm can still be applied (just modify "1." accordingly) and would still be advantageous. CU Andi |
From: Johannes S. <sch...@us...> - 2005-06-10 08:57:58
|
Am Mittwoch 08 Juni 2005 13:32 schrieb Andreas Beckermann: > On Wednesday 08 June 2005 09:30, Johannes Schmidt wrote: > > Am Mittwoch 08 Juni 2005 00:12 schrieb Andreas Beckermann: > > > Hi > > > there is a pretty huge problem with the accelerator implementation: > > > 1. create a button "button" and assign some action to it > > > 2. button->setText("&Foobar"); > > > 3. button->setVisible(false); > > > 4. press ALT+F > > > -> the action is executed, although the button is not visible anymore. > > > > > > This could be fixed e.g. by using a more object oriented design for the > > > accelerators: simply use the key events in the corresponding widgets, > > > instead of the input map of the root widget. > > > > Hmm, I have to admit that I didn't understand your suggestion. > > > > Only the focused widget gets key events, but the accelerators/shortcuts > > should work no matter which widget is focused. > > Hmm, yeah I did not think about that. > > What about this: when a KeyPressed event occurs: > 1. Send an Accel event down the whole hierarchy (starting at root) > 2. A widget that receives an Accel event checks whether it > 3. If the Accel event is not consumed: send a KeyPressed event just like > before > 4. If the Accel event is consumed: stop (do not send KeyPressed event) > > And while we're on it this could be slightly extended similar to the way it > works in Qt: before sending an Accel event, send an AccelOverride event. If > that one is consumed, skip the Accel event and send a KeyPressed event > directly. Otherwise continue as described above. > -> This way it could be avoided that Accels take the key press instead of > it being delivered to the correct widget (e.g. a lineedit which has the > focus). > > > An alternative to this procedure would be to continue using a global > Accelerator cellection, but also storing the associated widget, so that > widget->isVisible() and widget->isEnabled() can be checked. Doing that in > keybindingSlot() is indeed a temporary solution only, as this would have to > be done for every class that may provide accelerators. > With this global solution (which is what is used by Qt, btw) the previously > described event algorithm can still be applied (just modify "1." > accordingly) and would still be advantageous. Yet another non-trivial problem. Using something like the shortcut event like in QT seems to make sense. I am not yet sure about implementation details, but there should be some function within UWidget to add/remove shortcuts. And you should be able to add several sortcuts for one key stroke. Perhaps using some sort of shortcut manager. Secondly, I am pondering about a changing the behaviour of setVisible and setEnabled. A visible widget as child of an invisible widget doesn't really make sense. IMHO, hiding all child widgets when calling setVisible(false) is reasonable. I'd like to change it similar to this approach: http://doc.trolltech.com/4.0/qwidget.html#enabled-prop http://doc.trolltech.com/4.0/qwidget.html#visible-prop Regards, Johannes |
From: Andreas B. <b_...@gm...> - 2005-06-10 10:04:52
|
On Friday 10 June 2005 10:57, Johannes Schmidt wrote: [...] > Secondly, I am pondering about a changing the behaviour of > setVisible and setEnabled. Which change are you talking about? > A visible widget as child of an invisible widget doesn't really make sense. This depends on how you mean this. widget->setVisible(true); widgetParent->setVisible(false); _does_ make sense, i.e. widgetParent does _not_ change the "visible" settings of its children. Because then you can do widgetParent->setVisible(true); which makes both, the widgetParent and the widget visible again. So a child widget that has the "visible" property true although a parent is not visible _does_ make sense (same goes for enabled). Of course, if the visible property of a parent is false, all of its children should be hidden, too, no matter what ther visible property is set to. I believe this is how most (all?) widget toolkits work. So if you did mean this, I agree. > IMHO, hiding all child widgets when calling setVisible(false) is > reasonable. See above. If you mean by "hiding" not to show them, I agree. If you mean by it calling setVisible(false) on them, I disagree. > Regards, > Johannes CU Andi |
From: Johannes S. <sch...@us...> - 2005-06-10 11:01:49
|
Am Freitag 10 Juni 2005 12:04 schrieb Andreas Beckermann: > On Friday 10 June 2005 10:57, Johannes Schmidt wrote: > [...] > > > Secondly, I am pondering about a changing the behaviour of > > setVisible and setEnabled. > > Which change are you talking about? The change which was described below, especially when following the links. > > A visible widget as child of an invisible widget doesn't really make > > sense. > > This depends on how you mean this. > widget->setVisible(true); > widgetParent->setVisible(false); > _does_ make sense, i.e. widgetParent does _not_ change the "visible" > settings of its children. Because then you can do > widgetParent->setVisible(true); > which makes both, the widgetParent and the widget visible again. > So a child widget that has the "visible" property true although a parent is > not visible _does_ make sense (same goes for enabled). > > Of course, if the visible property of a parent is false, all of its > children should be hidden, too, no matter what ther visible property is set > to. I believe this is how most (all?) widget toolkits work. > So if you did mean this, I agree. Yes, the main reason is to get whether a widget is mapped to screen. > > IMHO, hiding all child widgets when calling setVisible(false) is > > reasonable. > > See above. If you mean by "hiding" not to show them, I agree. If you mean > by it calling setVisible(false) on them, I disagree. Well, as you always favor the way of QT, I thought that you'd like it. As described in the given APi docs, there should be some sort of force_invisible. But you could also introduce some new methods. Regards, Johannes |
From: Andreas B. <b_...@gm...> - 2005-06-12 14:26:32
|
On Friday 10 June 2005 13:01, Johannes Schmidt wrote: > Am Freitag 10 Juni 2005 12:04 schrieb Andreas Beckermann: > > On Friday 10 June 2005 10:57, Johannes Schmidt wrote: > > [...] > > > > > Secondly, I am pondering about a changing the behaviour of > > > setVisible and setEnabled. > > > > Which change are you talking about? > > The change which was described below, especially when following the links. I still don't see which change you mean The docs that you quoted state pretty much the current behaviour of libufo, except for the fact that the enabled property of libufo is mostly ignored currently. [...] > > > IMHO, hiding all child widgets when calling setVisible(false) is > > > reasonable. > > > > See above. If you mean by "hiding" not to show them, I agree. If you mean > > by it calling setVisible(false) on them, I disagree. > > Well, as you always favor the way of QT, I thought that you'd like it. > As described in the given APi docs, there should be some sort of > force_invisible. But you could also introduce some new methods. Really, I still fail to see your point, please explain further what you mean. Do you really want to make the behaviour of setVisible() turn into this: void UWidget::setVisible(bool e) { m_isVisible = e; for (all children) { child->setVisible(e); } } ? If so, then I think that is a _really_ bad idea. btw: Qt does _not_ do that. In Qt setVisible(false) actually _does_ hide all children, yes. But it does not call setVisible(false) on them, but implicitly hides them. This is the same how libufo behaves right now - if the parent is not visible, the children aren't either, not matter what m_isVisible looks like for them. Only if all parents are visible, m_isVisible does something. Same goes for setEnabled() (however this is mostly ignored by libufo atm). > Regards, > Johannes CU Andi |
From: Johannes S. <sch...@us...> - 2005-06-13 15:37:16
|
Am Sonntag 12 Juni 2005 16:26 schrieb Andreas Beckermann: > On Friday 10 June 2005 13:01, Johannes Schmidt wrote: > > Am Freitag 10 Juni 2005 12:04 schrieb Andreas Beckermann: > > > On Friday 10 June 2005 10:57, Johannes Schmidt wrote: > > > [...] > > > > > > > Secondly, I am pondering about a changing the behaviour of > > > > setVisible and setEnabled. > > > > > > Which change are you talking about? > > > > The change which was described below, especially when following the > > links. > > I still don't see which change you mean > The docs that you quoted state pretty much the current behaviour of libufo, > except for the fact that the enabled property of libufo is mostly ignored > currently. > [...] > > > > > IMHO, hiding all child widgets when calling setVisible(false) is > > > > reasonable. > > > > > > See above. If you mean by "hiding" not to show them, I agree. If you > > > mean by it calling setVisible(false) on them, I disagree. > > > > Well, as you always favor the way of QT, I thought that you'd like it. > > As described in the given APi docs, there should be some sort of > > force_invisible. But you could also introduce some new methods. > > Really, I still fail to see your point, please explain further what you > mean. Do you really want to make the behaviour of setVisible() turn into > this: void UWidget::setVisible(bool e) > { > m_isVisible = e; > for (all children) { > child->setVisible(e); > } > } > ? > > If so, then I think that is a _really_ bad idea. btw: Qt does _not_ do > that. In Qt setVisible(false) actually _does_ hide all children, yes. But > it does not call setVisible(false) on them, but implicitly hides them. This > is the same how libufo behaves right now - if the parent is not visible, > the children aren't either, not matter what m_isVisible looks like for > them. Only if all parents are visible, m_isVisible does something. > Same goes for setEnabled() (however this is mostly ignored by libufo atm). Let me quote http://doc.trolltech.com/4.0/qwidget.html#enabled-prop: "Disabling a widget implicitly disables all its children. Enabling respectively enables all child widgets unless they have been explicitly disabled." As for the visible property: In my last E-Mail I wrote that the intention is to get an easy way for querying whether a widget is mapped to screen. The user behaviour for setVisible remains the same (i.e. if you call setVisible(false) on a widget, it remains invisible even if the parent widget becomes visible). Regards, Johannes |
From: Andreas B. <b_...@gm...> - 2005-06-13 16:52:24
|
On Monday 13 June 2005 17:37, Johannes Schmidt wrote: [...] > Let me quote http://doc.trolltech.com/4.0/qwidget.html#enabled-prop: > "Disabling a widget implicitly disables all its children. Enabling > respectively enables all child widgets unless they have been explicitly > disabled." > > As for the visible property: > In my last E-Mail I wrote that the intention is to get an easy way for > querying whether a widget is mapped to screen. > The user behaviour for setVisible remains the same (i.e. if you call > setVisible(false) on a widget, it remains invisible even if the > parent widget becomes visible). Ah, so actually you do _not_ intend to change the behaviour of setVisible()/setEnabled() (that is what you wrote in the beginning!). You want to change isVisible() and isEnabled() only - well I don't really care about that :-) To me it is very important that widget->setVisible(true); parentOfWidget->setVisible(false); hides both widgets and parentOfWidget->setVisible(true); makes both of them visible again, whereas parentOfWidget->setVisible(false); widget->setVisible(false); parentOfWidget->setVisible(true); makes the parentOfWidget visible only, while widget remains hidden. Therefore I vote strongly against changing setVisible() in the way that I have shown in my last mail, as it makes setVisible() essentially useless to me (and anyone else who wants to create non-trivial GUIs). btw: the lack of the described behaviour in plib is one of the reasons why I am using libufo :-) > Regards, > Johannes CU Andi |
From: Johannes S. <sch...@us...> - 2005-06-14 09:15:41
|
Am Montag 13 Juni 2005 18:52 schrieb Andreas Beckermann: > On Monday 13 June 2005 17:37, Johannes Schmidt wrote: > > Let me quote http://doc.trolltech.com/4.0/qwidget.html#enabled-prop: > > "Disabling a widget implicitly disables all its children. Enabling > > respectively enables all child widgets unless they have been explicitly > > disabled." > > > > As for the visible property: > > In my last E-Mail I wrote that the intention is to get an easy way for > > querying whether a widget is mapped to screen. > > The user behaviour for setVisible remains the same (i.e. if you call > > setVisible(false) on a widget, it remains invisible even if the > > parent widget becomes visible). > > Ah, so actually you do _not_ intend to change the behaviour of > setVisible()/setEnabled() (that is what you wrote in the beginning!). You > want to change isVisible() and isEnabled() only Ouch, sorry, misunderstanding. Yes, setVisible should do the same thing, it's just a change to the internal handling (and what the is*() methods return). > - well I don't really care about that :-) Okay, then I will change that. I am happy that the communication works (even though it may take some time :) Regards, Johannes |