Code review comment for lp:~unity-2d-team/unity-2d/unity-2d_enable-super-numkey-combo

Revision history for this message
Paweł Stołowski (stolowski) wrote :

1. I think that the logic based on isX11Keysym in LauncherView::forwardNumericHotkey(), as well as Hotkey constructor are good candidates for introducing two simple classes inherited from HotKey: NumericHotkey and NumpadNumHotKey that hide x11keysm-based logic. They would inherit from Hotkey and from a common abstract class that provides index() interface, hinding index = key - .... calculation/comparison logic. I don't insist, but I prefer to avoid leaking of logic outside of classes and use polymorphic classes instead.

2. I think HotkeyMonitor::getHotkeyFor(Qt::Key ...) should check for equality of isX11keysym flag, and not only key and modifiers values.

review: Needs Fixing

« Back to merge proposal