Review Request: Should automatically assign an accelerator to a checkable group box

[Christoph Feck]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3084/#review4343
-----------------------------------------------------------

Ship it!


Looks good, thanks.

I guess you lowered priority to 20 so that the algorithm will very likely not alter the assignments compared to the old implementation. I could not test if this is the case for every application, so I would suggest to commit it to trunk so that we can test it better.

- Christoph


On 2010-03-02 14:50:06, Jonathan Marten wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3084/
> -----------------------------------------------------------
> 
> (Updated 2010-03-02 14:50:06)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> The automatic assignment of accelerators to widgets does not assign a shortcut to
> a checkable group box.  For a standard group box an accelerator is indeed pointless,
> but it is useful for a checkable group box to allow keyboard interaction (it can be
> considered equivalent to a QCheckBox).
> 
> 
> This addresses bug 229135.
>     https://bugs.kde.org/show_bug.cgi?id=229135
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kdeui/shortcuts/kacceleratormanager.cpp 1091671 
>   /trunk/KDE/kdelibs/kdeui/shortcuts/kacceleratormanager_private.h 1091671 
> 
> Diff: http://reviewboard.kde.org/r/3084/diff
> 
> 
> Testing
> -------
> 
> Checked with these changes against my application (KDE4 port of Kooka) which uses a
> number of normal and checkable group boxes with no existing accelerators defined.
> Checkable boxes are assigned an accelerator, whereas the normal ones are left alone.
> 
> Also checked against Konqueror's preferences (which already has some non-checkable group
> boxes with explicit accelerators, these are left alone) and systemsettings.
> 
> 
> Thanks,
> 
> Jonathan
> 
>