Code review comment for lp:~zsombi/ubuntu-ui-toolkit/mnemonics

Revision history for this message
Cris Dywan (kalikiana) wrote :

- m_mouseAttached(false)
47 + m_mouseAttached(true),

The default must not change.

// FIXME we should only do this if we have HW keyboard attached
+ if (QuickUtils::instance().keyboardAttached()) {

The comment doesn't make sense - you have an API right there. Better make it a FIXME with a reference to lp#.

+ // FIXME: do this with a proper API
+ connect(&QuickUtils::instance(), &QuickUtils::keyboardAttachedChanged,
+ this, &UCAction::onKeyboardAttached);

Again, add a FIXME with a reference to lp# matching the above.

        function test_mnemonic_data() {

How about more corner cases that could potentially crash? Such as "Hit& Space", "Paste &&&Proceed", "Jump &".

review: Needs Fixing

« Back to merge proposal