QAbstractSlider: fix invertedControls having no effect for left/right keys

There was a comment in the code that said:

// It seems we need to use invertedAppearance for Left and right, otherwise, things look weird.

It's not clear what that was referring to, but in its current state,
a slider with invertedControls set to true will not behave as expected:
pressing the left arrow key will decrease its value instead of increasing it,
and vice versa for the right arrow key.

As stated in the documentation (and by its name), invertedAppearance only
controls the appearance of the slider, and not the effect of key events.

Remove the comment and use invertedControls instead.

Change-Id: I13296cbda9244413978ef0d7f0856065f74fd0bf
Fixes: QTBUG-25988
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
This commit is contained in:
Mitch Curtis 2018-11-30 16:53:12 +01:00
parent 35602d75d3
commit 32fd79a20f
2 changed files with 491 additions and 8 deletions

View File

@ -829,7 +829,6 @@ void QAbstractSlider::keyPressEvent(QKeyEvent *ev)
break;
#endif
// It seems we need to use invertedAppearance for Left and right, otherwise, things look weird.
case Qt::Key_Left:
#ifdef QT_KEYPAD_NAVIGATION
// In QApplication::KeypadNavigationDirectional, we want to change the slider
@ -848,9 +847,9 @@ void QAbstractSlider::keyPressEvent(QKeyEvent *ev)
else
#endif
if (isRightToLeft())
action = d->invertedAppearance ? SliderSingleStepSub : SliderSingleStepAdd;
action = d->invertedControls ? SliderSingleStepSub : SliderSingleStepAdd;
else
action = !d->invertedAppearance ? SliderSingleStepSub : SliderSingleStepAdd;
action = !d->invertedControls ? SliderSingleStepSub : SliderSingleStepAdd;
break;
case Qt::Key_Right:
#ifdef QT_KEYPAD_NAVIGATION
@ -868,9 +867,9 @@ void QAbstractSlider::keyPressEvent(QKeyEvent *ev)
else
#endif
if (isRightToLeft())
action = d->invertedAppearance ? SliderSingleStepAdd : SliderSingleStepSub;
action = d->invertedControls ? SliderSingleStepAdd : SliderSingleStepSub;
else
action = !d->invertedAppearance ? SliderSingleStepAdd : SliderSingleStepSub;
action = !d->invertedControls ? SliderSingleStepAdd : SliderSingleStepSub;
break;
case Qt::Key_Up:
#ifdef QT_KEYPAD_NAVIGATION

View File

@ -485,6 +485,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step down once 1, horizontal, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step down once 1, horizontal, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step down once 1, horizontal, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -507,6 +529,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step down once 1, vertical, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step down once 1, vertical, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step down once 1, vertical, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -531,6 +575,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step up once 2, horizontal, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step up once 2, horizontal, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step up once 2, horizontal, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -553,6 +619,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step up once 2, vertical, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step up once 2, vertical, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step up once 2, vertical, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -577,6 +665,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step left once 2, horizontal, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step left once 2, horizontal, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step left once 2, horizontal, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -599,6 +709,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step left once 2, vertical, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step left once 2, vertical, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step left once 2, vertical, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -623,6 +755,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step right once 2, horizontal, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step right once 2, horizontal, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step right once 2, horizontal, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -645,6 +799,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step right once 2, vertical, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Step right once 2, vertical, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 0 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Step right once 2, vertical, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -654,7 +830,7 @@ void tst_QAbstractSlider::keyPressed_data()
<< true // inverted appearance
<< true // inverted controls
<< list // key sequence
<< 7; // expected position
<< 7; // expected position
list = QList<Qt::Key>();
list << Qt::Key_PageDown;
@ -667,7 +843,29 @@ void tst_QAbstractSlider::keyPressed_data()
<< false// inverted appearance
<< false// inverted controls
<< list // key sequence
<< 7; // expected position
<< 7; // expected position
QTest::newRow("Page down once, horizontal, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Page down once, horizontal, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Page down once, horizontal, both inverted")
<< 10 // initial position
@ -689,7 +887,29 @@ void tst_QAbstractSlider::keyPressed_data()
<< false// inverted appearance
<< false// inverted controls
<< list // key sequence
<< 7; // expected position
<< 7; // expected position
QTest::newRow("Page down once, vertical, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Page down once, vertical, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Page down once, vertical, both inverted")
<< 10 // initial position
@ -715,6 +935,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 13; // expected position
QTest::newRow("Page up once, horizontal, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Page up once, horizontal, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Page up once, horizontal, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -737,6 +979,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 13; // expected position
QTest::newRow("Page up once, vertical, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 13; // expected position
QTest::newRow("Page up once, vertical, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 7; // expected position
QTest::newRow("Page up once, vertical, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -762,6 +1026,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 50; // expected position
QTest::newRow("Symmetric seq, horizontal, appearance inverted")
<< 50 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 50; // expected position
QTest::newRow("Symmetric seq, horizontal, controls inverted")
<< 50 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 50; // expected position
QTest::newRow("Symmetric seq, horizontal, both inverted")
<< 50 // initial position
<< 0 // minimum
@ -784,6 +1070,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 50; // expected position
QTest::newRow("Symmetric seq, vertical, appearance inverted")
<< 50 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 50; // expected position
QTest::newRow("Symmetric seq, vertical, controls inverted")
<< 50 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 50; // expected position
QTest::newRow("Symmetric seq, vertical, both inverted")
<< 50 // initial position
<< 0 // minimum
@ -808,6 +1116,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 0; // expected position
QTest::newRow("Home, horizontal, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 0; // expected position
QTest::newRow("Home, horizontal, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 0; // expected position
QTest::newRow("Home, horizontal, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -830,6 +1160,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 0; // expected position
QTest::newRow("Home, vertical, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 0; // expected position
QTest::newRow("Home, vertical, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 0; // expected position
QTest::newRow("Home, vertical, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -854,6 +1206,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 100; // expected position
QTest::newRow("End, horizontal, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 100; // expected position
QTest::newRow("End, horizontal, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 100; // expected position
QTest::newRow("End, horizontal, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -876,6 +1250,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 100; // expected position
QTest::newRow("End, vertical, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 100; // expected position
QTest::newRow("End, vertical, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 0 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 100; // expected position
QTest::newRow("End, vertical, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -900,6 +1296,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 100; // expected position
QTest::newRow("Past end, horizontal, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 100; // expected position
QTest::newRow("Past end, horizontal, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 97; // expected position
QTest::newRow("Past end, horizontal, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -922,6 +1340,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 100; // expected position
QTest::newRow("Past end, vertical, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 100; // expected position
QTest::newRow("Past end, vertical, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 97; // expected position
QTest::newRow("Past end, vertical, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -946,6 +1386,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 0; // expected position
QTest::newRow("Past home, horizontal, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< true // inverted appearance
<< false// inverted controls
<< list // key sequence
<< 0; // expected position
QTest::newRow("Past home, horizontal, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 3; // expected position
QTest::newRow("Past home, horizontal, both inverted")
<< 10 // initial position
<< 0 // minimum
@ -968,6 +1430,28 @@ void tst_QAbstractSlider::keyPressed_data()
<< list // key sequence
<< 0; // expected position
QTest::newRow("Past home, vertical, appearance inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< false// inverted appearance
<< false// inverted controls
<< list // key sequence
<< 0; // expected position
QTest::newRow("Past home, vertical, controls inverted")
<< 10 // initial position
<< 0 // minimum
<< 100 // maximum
<< 3 // single step size
<< 3 // page step size
<< false// inverted appearance
<< true // inverted controls
<< list // key sequence
<< 3; // expected position
QTest::newRow("Past home, vertical, both inverted")
<< 10 // initial position
<< 0 // minimum