diff --git a/Common/UI/PopupScreens.cpp b/Common/UI/PopupScreens.cpp index 65e5406ebb..678810405a 100644 --- a/Common/UI/PopupScreens.cpp +++ b/Common/UI/PopupScreens.cpp @@ -1053,15 +1053,15 @@ void AbstractChoiceWithValueDisplay::GetContentDimensionsBySpec(const UIContext void AbstractChoiceWithValueDisplay::Draw(UIContext &dc) { Style style = dc.GetTheme().itemStyle; - if (!IsEnabled()) { - style = dc.GetTheme().itemDisabledStyle; - } if (HasFocus()) { style = dc.GetTheme().itemFocusedStyle; } if (down_) { style = dc.GetTheme().itemDownStyle; } + if (!IsEnabled()) { + style = dc.GetTheme().itemDisabledStyle; + } int paddingX = 12; dc.SetFontStyle(dc.GetTheme().uiFont); diff --git a/Common/UI/View.cpp b/Common/UI/View.cpp index ead2069343..0b3ae2f84b 100644 --- a/Common/UI/View.cpp +++ b/Common/UI/View.cpp @@ -219,6 +219,11 @@ void Clickable::DrawBG(UIContext &dc, const Style &style, float radius) { if (highlighted_) { dc.Draw()->DrawRoundRect(bounds_, radius, 0xFFFFFFFF, RoundRectStyle::OutlineThin, roundCorners_); } + // Disabled items can be focused (to read them), but keep their disabled background. + // Outline them in the focus color instead. + if (HasFocus() && !IsEnabled()) { + dc.Draw()->DrawRoundRect(bounds_, radius, dc.GetTheme().itemFocusedStyle.background.color, RoundRectStyle::OutlineThin, roundCorners_); + } } void Clickable::ClickInternal() { @@ -426,15 +431,15 @@ ClickableItem::ClickableItem(LayoutParams *layoutParams) : Clickable(layoutParam void ClickableItem::Draw(UIContext &dc) { Style style = dc.GetTheme().itemStyle; - if (!IsEnabled()) { - style = dc.GetTheme().itemDisabledStyle; - } if (HasFocus()) { style = dc.GetTheme().itemFocusedStyle; } if (down_) { style = dc.GetTheme().itemDownStyle; } + if (!IsEnabled()) { + style = dc.GetTheme().itemDisabledStyle; + } DrawBG(dc, style, dc.GetTheme().choiceRoundRadius); } @@ -827,19 +832,21 @@ void CheckBox::Draw(UIContext &dc) { if (!IsEnabled()) { style = dc.GetTheme().itemDisabledStyle; } + // A focused disabled checkbox keeps its disabled look, DrawBG outlines it. + const bool focused = HasFocus() && IsEnabled(); ImageID image = Toggled() ? dc.GetTheme().checkOn : dc.GetTheme().checkOff; // In image mode, light up instead of showing a checkbox. if (imageID_.isValid()) { image = imageID_; if (Toggled()) { - if (HasFocus()) { + if (focused) { style = dc.GetTheme().itemDownStyle; } else { style = dc.GetTheme().itemFocusedStyle; } } else { - if (HasFocus()) { + if (focused) { style = dc.GetTheme().itemDownStyle; } else { style = dc.GetTheme().itemStyle; @@ -851,7 +858,7 @@ void CheckBox::Draw(UIContext &dc) { } } else { - if (HasFocus()) { + if (focused) { style = dc.GetTheme().itemFocusedStyle; } if (down_) { diff --git a/Common/UI/ViewGroup.cpp b/Common/UI/ViewGroup.cpp index f35ed4b6d7..7ab1eebd3a 100644 --- a/Common/UI/ViewGroup.cpp +++ b/Common/UI/ViewGroup.cpp @@ -134,8 +134,15 @@ bool ViewGroup::Key(const KeyInput &input) { bool ret = false; for (View *view : views_) { // TODO: If there is a transformation active, transform input coordinates accordingly. - if (view->GetVisibility() == V_VISIBLE) - ret = ret || view->Key(input); + if (view->GetVisibility() != V_VISIBLE) { + continue; + } + // Disabled views can have focus, but must not be activated. Groups pass keys on to + // their children regardless, like CollectTabOrder. + if (!view->IsEnabled() && !view->IsViewGroup()) { + continue; + } + ret = ret || view->Key(input); } return ret; } @@ -407,7 +414,8 @@ bool IsBetterFocusCandidate(const Bounds &origin, const Point2D &originPos, cons } static bool CanTakeFocus(const View *view) { - return view->CanBeFocused() && view->IsEnabled() && view->GetVisibility() == V_VISIBLE; + // Disabled views can be focused too (they ignore keys, see Key()). + return view->CanBeFocused() && view->GetVisibility() == V_VISIBLE; } float GetTargetScore(const Point2D &originPos, int originIndex, const View *origin, const View *destination, FocusMove direction) { @@ -554,11 +562,11 @@ void ViewGroup::CollectTabOrder(std::vector *outViews) const { // A group can be focusable itself (rare), in which case it's a stop and we still // descend into it - same as arrow navigation, which considers both. ViewGroup *vg = static_cast(view); - if (vg->CanBeFocused() && vg->IsEnabled()) { + if (vg->CanBeFocused()) { outViews->push_back(vg); } vg->CollectTabOrder(outViews); - } else if (view->CanBeFocused() && view->IsEnabled()) { + } else if (view->CanBeFocused()) { outViews->push_back(view); } } diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 73cbf99b69..59b014aebe 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -3985,6 +3985,7 @@ bool TestUITabOrder() { // Nested groups are flattened in place, in order. LinearLayout *inner = root.Add(new LinearLayout(ORIENT_HORIZONTAL)); Choice *b = inner->Add(new Choice("b")); + // Disabled items are stops too, they just can't be activated. Choice *disabled = inner->Add(new Choice("disabled")); disabled->SetEnabled(false); @@ -4001,23 +4002,26 @@ bool TestUITabOrder() { std::vector order; root.CollectTabOrder(&order); - EXPECT_EQ_INT((int)order.size(), 3); + EXPECT_EQ_INT((int)order.size(), 4); EXPECT_TRUE(order[0] == a); EXPECT_TRUE(order[1] == b); - EXPECT_TRUE(order[2] == c); + EXPECT_TRUE(order[2] == disabled); + EXPECT_TRUE(order[3] == c); // Tab walks forwards and wraps at the end, Shift+Tab does the reverse. EXPECT_TRUE(FindTabOrderNeighbor(&root, a, FocusMove::NEXT) == b); - EXPECT_TRUE(FindTabOrderNeighbor(&root, b, FocusMove::NEXT) == c); + EXPECT_TRUE(FindTabOrderNeighbor(&root, b, FocusMove::NEXT) == disabled); + EXPECT_TRUE(FindTabOrderNeighbor(&root, disabled, FocusMove::NEXT) == c); EXPECT_TRUE(FindTabOrderNeighbor(&root, c, FocusMove::NEXT) == a); - EXPECT_TRUE(FindTabOrderNeighbor(&root, c, FocusMove::PREV) == b); + EXPECT_TRUE(FindTabOrderNeighbor(&root, c, FocusMove::PREV) == disabled); + EXPECT_TRUE(FindTabOrderNeighbor(&root, disabled, FocusMove::PREV) == b); EXPECT_TRUE(FindTabOrderNeighbor(&root, b, FocusMove::PREV) == a); EXPECT_TRUE(FindTabOrderNeighbor(&root, a, FocusMove::PREV) == c); // A view that has gone away (or was never a stop) doesn't stall navigation - it starts // from whichever end we're heading towards. - EXPECT_TRUE(FindTabOrderNeighbor(&root, disabled, FocusMove::NEXT) == a); - EXPECT_TRUE(FindTabOrderNeighbor(&root, disabled, FocusMove::PREV) == c); + EXPECT_TRUE(FindTabOrderNeighbor(&root, invisible, FocusMove::NEXT) == a); + EXPECT_TRUE(FindTabOrderNeighbor(&root, invisible, FocusMove::PREV) == c); EXPECT_TRUE(FindTabOrderNeighbor(&root, nullptr, FocusMove::NEXT) == a); // With a single stop, both directions land back on it, and with none there's nothing to do. @@ -4030,6 +4034,42 @@ bool TestUITabOrder() { empty.Add(new TextView("just a label")); EXPECT_TRUE(FindTabOrderNeighbor(&empty, nullptr, FocusMove::NEXT) == nullptr); + // A focused disabled item ignores the accept key, an enabled one clicks. + { + const std::vector oldConfirm = confirmKeys; + const std::vector oldCancel = cancelKeys; + SetConfirmCancelKeys({ InputMapping(DEVICE_ID_KEYBOARD, NKCODE_ENTER) }, { InputMapping(DEVICE_ID_KEYBOARD, NKCODE_ESCAPE) }); + + LinearLayout keyRoot(ORIENT_VERTICAL); + Choice *enabledChoice = keyRoot.Add(new Choice("enabled")); + Choice *disabledChoice = keyRoot.Add(new Choice("disabled")); + disabledChoice->SetEnabled(false); + int enabledClicks = 0; + int disabledClicks = 0; + enabledChoice->OnClick.Add([&](EventParams &) { enabledClicks++; }); + disabledChoice->OnClick.Add([&](EventParams &) { disabledClicks++; }); + + auto press = [&](View *focus) { + SetFocusedView(focus, FocusFlags::CAUSE_OTHER); + KeyInput key{}; + key.deviceId = DEVICE_ID_KEYBOARD; + key.keyCode = NKCODE_ENTER; + key.flags = KeyInputFlags::DOWN; + keyRoot.Key(key); + key.flags = KeyInputFlags::UP; + keyRoot.Key(key); + }; + press(disabledChoice); + press(enabledChoice); + // Clicks are queued. + DispatchEvents(); + SetFocusedView(nullptr, FocusFlags::CAUSE_OTHER); + SetConfirmCancelKeys(oldConfirm, oldCancel); + + EXPECT_EQ_INT(disabledClicks, 0); + EXPECT_EQ_INT(enabledClicks, 1); + } + return true; } @@ -4108,16 +4148,24 @@ bool TestUIFocusNavigation() { // Things that can't take focus are passed over, also inside nested groups. LinearLayout root(ORIENT_VERTICAL); Choice *origin = add(&root, 0, 0, 100, 50); - Choice *disabled = add(&root, 0, 60, 100, 50); - disabled->SetEnabled(false); LinearLayout *inner = root.Add(new LinearLayout(ORIENT_VERTICAL)); - Choice *hidden = add(inner, 0, 120, 100, 50); + Choice *hidden = add(inner, 0, 60, 100, 50); hidden->SetVisibility(V_INVISIBLE); - Choice *last = add(inner, 0, 180, 100, 50); + Choice *last = add(inner, 0, 120, 100, 50); EXPECT_TRUE(move(&root, origin, FocusMove::DOWN) == last); EXPECT_TRUE(move(&root, last, FocusMove::UP) == origin); } + { + // Disabled items can take focus (they ignore activation instead). + LinearLayout root(ORIENT_VERTICAL); + Choice *origin = add(&root, 0, 0, 100, 50); + Choice *disabled = add(&root, 0, 60, 100, 50); + disabled->SetEnabled(false); + EXPECT_TRUE(move(&root, origin, FocusMove::DOWN) == disabled); + EXPECT_TRUE(move(&root, disabled, FocusMove::UP) == origin); + } + return true; }