diff --git a/Common/UI/Root.cpp b/Common/UI/Root.cpp index 564381e87c..18de799d4e 100644 --- a/Common/UI/Root.cpp +++ b/Common/UI/Root.cpp @@ -1,3 +1,4 @@ +#include #include #include "ppsspp_config.h" @@ -10,7 +11,7 @@ namespace UI { -static std::vector focusMoves; +static std::vector focusMoves; extern bool focusForced; static View *focusedView; @@ -33,6 +34,9 @@ struct HeldKey { InputKeyCode key; InputDeviceID deviceId; double triggerTime; + // What the key meant when it went down. Kept here because a repeat has to move the same way + // the original press did, and modifiers (Shift+Tab) aren't part of the synthesized repeat. + FocusMove move; // Ignores startTime bool operator <(const HeldKey &other) const { @@ -150,6 +154,29 @@ void LayoutViewHierarchy(const UIContext &dc, const UI::Margins &rootMargins, Vi root->Layout(); } +// Tab order navigation. Unlike the directional moves, this doesn't look at where things ended up +// on screen at all - it just walks the hierarchy in order, which is why it behaves predictably in +// layouts where "what's to the right of this" has no good answer. +View *FindTabOrderNeighbor(ViewGroup *root, View *focusedView, FocusMove direction) { + std::vector order; + root->CollectTabOrder(&order); + if (order.empty()) { + return nullptr; + } + + const int step = direction == FocusMove::NEXT ? 1 : (int)order.size() - 1; + + // The focused view not being in the list is normal - it can have been hidden or disabled + // since it got focus. Start from one end rather than giving up. + auto iter = std::find(order.begin(), order.end(), focusedView); + if (iter == order.end()) { + return direction == FocusMove::NEXT ? order.front() : order.back(); + } + + const size_t index = iter - order.begin(); + return order[(index + step) % order.size()]; +} + static void MoveFocus(ViewGroup *root, FocusMove direction) { View *focusedView = GetFocusedView(); if (!focusedView) { @@ -162,12 +189,24 @@ static void MoveFocus(ViewGroup *root, FocusMove direction) { return; } - NeighborResult neigh = root->FindNeighbor(focusedView, direction, NeighborResult()); - if (neigh.view) { - neigh.view->SetFocus(FocusFlags::CAUSE_FOCUS_MOVE); - root->SubviewFocused(neigh.view); + View *dest; + if (direction == FocusMove::NEXT || direction == FocusMove::PREV) { + dest = FindTabOrderNeighbor(root, focusedView, direction); + if (dest == focusedView) { + // Tabbing around a single stop. Don't re-fire focus events or play a sound for it. + // The directional moves below deliberately can land on the origin - FindScrollNeighbor + // considers it a candidate - so this only applies here. + dest = nullptr; + } + } else { + dest = root->FindNeighbor(focusedView, direction, NeighborResult()).view; + } - // INFO_LOG(Log::UI, "Focus moved from %s to %s", focusedView->DescribeText().c_str(), neigh.view->DescribeText().c_str()); + if (dest) { + dest->SetFocus(FocusFlags::CAUSE_FOCUS_MOVE); + root->SubviewFocused(dest); + + // INFO_LOG(Log::UI, "Focus moved from %s to %s", focusedView->DescribeText().c_str(), dest->DescribeText().c_str()); PlayUISound(UISound::SELECT); } @@ -183,12 +222,29 @@ void PlayUISound(UISound sound) { } } -bool IsScrollKey(const KeyInput &input) { - switch (input.keyCode) { - case NKCODE_PAGE_UP: - case NKCODE_PAGE_DOWN: - case NKCODE_MOVE_HOME: - case NKCODE_MOVE_END: +// Which way, if any, this key moves the focus. DPad keys are remappable, so they're matched +// through IsDPadKey rather than by keycode here. +static bool KeyToFocusMove(const KeyInput &key, FocusMove *move) { + if (IsDPadKey(key)) { + switch (key.keyCode) { + case NKCODE_DPAD_LEFT: *move = FocusMove::LEFT; return true; + case NKCODE_DPAD_RIGHT: *move = FocusMove::RIGHT; return true; + case NKCODE_DPAD_UP: *move = FocusMove::UP; return true; + case NKCODE_DPAD_DOWN: *move = FocusMove::DOWN; return true; + default: break; + } + } + switch (key.keyCode) { + case NKCODE_PAGE_UP: *move = FocusMove::PREV_PAGE; return true; + case NKCODE_PAGE_DOWN: *move = FocusMove::NEXT_PAGE; return true; + case NKCODE_MOVE_HOME: *move = FocusMove::FIRST; return true; + case NKCODE_MOVE_END: *move = FocusMove::LAST; return true; + case NKCODE_TAB: + // Ctrl+Tab switches tabs rather than moving focus - see ChoiceStrip::Key. + if (key.flags & KeyInputFlags::ModCtrl) { + return false; + } + *move = (key.flags & KeyInputFlags::ModShift) ? FocusMove::PREV : FocusMove::NEXT; return true; default: return false; @@ -199,12 +255,14 @@ KeyEventResult KeyEventToFocusMoves(const KeyInput &key) { KeyEventResult retval = KeyEventResult::PASS_THROUGH; // Ignore repeats for focus moves. if ((key.flags & KeyInputFlags::DOWN) && !(key.flags & KeyInputFlags::IS_REPEAT)) { - if (IsDPadKey(key) || IsScrollKey(key)) { + FocusMove move; + if (KeyToFocusMove(key, &move)) { // Let's only repeat DPAD initially. HeldKey hk; hk.key = key.keyCode; hk.deviceId = key.deviceId; hk.triggerTime = time_now_d() + repeatDelay; + hk.move = move; // Check if the key is already held. If it is, ignore it. This is to avoid // multiple key repeat mechanisms colliding. @@ -213,7 +271,7 @@ KeyEventResult KeyEventToFocusMoves(const KeyInput &key) { } heldKeys.insert(hk); - focusMoves.push_back(key.keyCode); + focusMoves.push_back(move); retval = KeyEventResult::ACCEPT; } } @@ -224,6 +282,7 @@ KeyEventResult KeyEventToFocusMoves(const KeyInput &key) { hk.key = key.keyCode; hk.deviceId = key.deviceId; hk.triggerTime = 0.0; // irrelevant + hk.move = FocusMove::NEXT; // irrelevant, not part of the comparison if (heldKeys.find(hk) != heldKeys.end()) { heldKeys.erase(hk); retval = KeyEventResult::ACCEPT; @@ -351,7 +410,7 @@ restart: key.flags = KeyInputFlags::DOWN; KeyEvent(key, root); - focusMoves.push_back(key.keyCode); + focusMoves.push_back(iter->move); // Cannot modify the current item when looping over a set, so let's do this instead. HeldKey hk = *iter; @@ -385,18 +444,8 @@ DialogResult UpdateViewHierarchy(ViewGroup *root, bool canEnableFocusMovement) { } root->SubviewFocused(GetFocusedView()); } else { - for (size_t i = 0; i < focusMoves.size(); i++) { - switch (focusMoves[i]) { - case NKCODE_DPAD_LEFT: MoveFocus(root, FocusMove::LEFT); break; - case NKCODE_DPAD_RIGHT: MoveFocus(root, FocusMove::RIGHT); break; - case NKCODE_DPAD_UP: MoveFocus(root, FocusMove::UP); break; - case NKCODE_DPAD_DOWN: MoveFocus(root, FocusMove::DOWN); break; - case NKCODE_PAGE_UP: MoveFocus(root, FocusMove::PREV_PAGE); break; - case NKCODE_PAGE_DOWN: MoveFocus(root, FocusMove::NEXT_PAGE); break; - case NKCODE_MOVE_HOME: MoveFocus(root, FocusMove::FIRST); break; - case NKCODE_MOVE_END: MoveFocus(root, FocusMove::LAST); break; - case NKCODE_TAB: MoveFocus(root, FocusMove::NEXT); break; - } + for (FocusMove move : focusMoves) { + MoveFocus(root, move); } } } diff --git a/Common/UI/Root.h b/Common/UI/Root.h index e5d341503a..d975476f2f 100644 --- a/Common/UI/Root.h +++ b/Common/UI/Root.h @@ -10,6 +10,7 @@ namespace UI { struct Margins; enum class FocusFlags; +enum class FocusMove; // The ONLY global is the currently focused item. // Can be and often is null. @@ -25,6 +26,11 @@ DialogResult DispatchEvents(); class ViewGroup; +// Where Tab (FocusMove::NEXT) and Shift+Tab (FocusMove::PREV) move the focus to, wrapping around +// at the ends. See ViewGroup::CollectTabOrder for what counts as a stop and in what order. +// nullptr if there's nothing focusable at all. Exposed mainly so it can be tested directly. +View *FindTabOrderNeighbor(ViewGroup *root, View *focusedView, FocusMove direction); + void LayoutViewHierarchy(const UIContext &dc, const UI::Margins &rootMargins, UI::ViewGroup *root, ViewLayoutMode layoutMode, bool immersiveMode); DialogResult UpdateViewHierarchy(ViewGroup *root, bool canEnableFocusMovement = true); diff --git a/Common/UI/ViewGroup.cpp b/Common/UI/ViewGroup.cpp index 6f96a37ba0..c3b4edde97 100644 --- a/Common/UI/ViewGroup.cpp +++ b/Common/UI/ViewGroup.cpp @@ -489,6 +489,27 @@ NeighborResult ViewGroup::FindNeighbor(View *view, FocusMove direction, Neighbor } } +void ViewGroup::CollectTabOrder(std::vector *outViews) const { + for (View *view : views_) { + // Gate on visibility only, like Key/Touch/Axis do - a container being disabled doesn't + // stop its children from being interactive elsewhere, so it shouldn't here either. + if (view->GetVisibility() != V_VISIBLE) { + continue; + } + if (view->IsViewGroup()) { + // 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()) { + outViews->push_back(vg); + } + vg->CollectTabOrder(outViews); + } else if (view->CanBeFocused() && view->IsEnabled()) { + outViews->push_back(view); + } + } +} + NeighborResult ViewGroup::FindScrollNeighbor(View *view, const Point2D &target, FocusMove direction, NeighborResult best) { if (!IsEnabled()) return best; diff --git a/Common/UI/ViewGroup.h b/Common/UI/ViewGroup.h index b929545b38..47263323b5 100644 --- a/Common/UI/ViewGroup.h +++ b/Common/UI/ViewGroup.h @@ -66,6 +66,12 @@ public: NeighborResult FindNeighbor(View *view, FocusMove direction, NeighborResult best); virtual NeighborResult FindScrollNeighbor(View *view, const Point2D &target, FocusMove direction, NeighborResult best); + // Appends the views Tab/Shift+Tab step through, in the order they were added, depth first - + // so the order follows the hierarchy rather than the geometry, which is what makes tabbing + // predictable in a layout that arrow navigation has to guess its way around. + // Unlike FindNeighbor, this doesn't need layout to have taken place. + virtual void CollectTabOrder(std::vector *outViews) const; + bool CanBeFocused() const override { return false; } bool IsViewGroup() const override { return true; } bool ContainsSubview(const View *view) const override; diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 9b98a3c3f8..fdc79c953f 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -99,6 +99,9 @@ #include "Core/Util/BlockAllocator.h" #include "Core/Debugger/Breakpoints.h" #include "Core/Debugger/SymbolMap.h" +#include "Common/UI/Root.h" +#include "Common/UI/View.h" +#include "Common/UI/ViewGroup.h" #include "Core/Debugger/MemBlockInfo.h" #include "Core/FileSystems/ISOFileSystem.h" #include "Core/MemMap.h" @@ -2855,6 +2858,70 @@ bool TestVFS(); bool TestZipSlip(); bool TestLzrc(); bool TestDemangle(); + +// Tab/Shift+Tab focus navigation walks the view hierarchy in declaration order rather than by +// geometry, so what it does is entirely determined by CollectTabOrder - which is worth pinning +// down, since the interesting cases (nesting, hidden tabs, disabled items) are all structural. +bool TestUITabOrder() { + using namespace UI; + + LinearLayout root(ORIENT_VERTICAL); + + // A label is not a tab stop, but the item after it is. + root.Add(new TextView("label")); + Choice *a = root.Add(new Choice("a")); + + // Nested groups are flattened in place, in order. + LinearLayout *inner = root.Add(new LinearLayout(ORIENT_HORIZONTAL)); + Choice *b = inner->Add(new Choice("b")); + Choice *disabled = inner->Add(new Choice("disabled")); + disabled->SetEnabled(false); + + // A hidden subtree is skipped whole - this is how the inactive tabs of a TabHolder, + // which are V_GONE rather than removed, stay out of the way. + LinearLayout *hidden = root.Add(new LinearLayout(ORIENT_VERTICAL)); + hidden->SetVisibility(V_GONE); + hidden->Add(new Choice("hidden")); + + root.Add(new Spacer()); + Choice *c = root.Add(new Choice("c")); + Choice *invisible = root.Add(new Choice("invisible")); + invisible->SetVisibility(V_INVISIBLE); + + std::vector order; + root.CollectTabOrder(&order); + EXPECT_EQ_INT((int)order.size(), 3); + EXPECT_TRUE(order[0] == a); + EXPECT_TRUE(order[1] == b); + EXPECT_TRUE(order[2] == 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, c, FocusMove::NEXT) == a); + EXPECT_TRUE(FindTabOrderNeighbor(&root, c, 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, nullptr, FocusMove::NEXT) == a); + + // With a single stop, both directions land back on it, and with none there's nothing to do. + LinearLayout one(ORIENT_VERTICAL); + Choice *only = one.Add(new Choice("only")); + EXPECT_TRUE(FindTabOrderNeighbor(&one, only, FocusMove::NEXT) == only); + EXPECT_TRUE(FindTabOrderNeighbor(&one, only, FocusMove::PREV) == only); + + LinearLayout empty(ORIENT_VERTICAL); + empty.Add(new TextView("just a label")); + EXPECT_TRUE(FindTabOrderNeighbor(&empty, nullptr, FocusMove::NEXT) == nullptr); + + return true; +} + bool TestTextureReplacer(); TestItem availableTests[] = { @@ -2924,6 +2991,7 @@ TestItem availableTests[] = { TEST_ITEM(Lzrc), TEST_ITEM(Demangle), TEST_ITEM(TextureReplacer), + TEST_ITEM(UITabOrder), }; int main(int argc, const char *argv[]) {