UI: Tab and Shift+Tab move focus through the view hierarchy

Unlike the directional moves, this doesn't look at where anything ended up on
screen - it walks the hierarchy in the order views were added, flattening nested
groups in place. That's what makes it predictable in the layouts where "what's
to the right of this" has no good answer.

A view is a stop if it's focusable and enabled, the same test the directional
moves apply, so the two agree on what's reachable. Hidden subtrees are skipped
whole, which is what keeps a TabHolder's inactive tabs - V_GONE rather than
removed - out of the order without any special casing. Containers are gated on
visibility only, not enabled, matching Key/Touch/Axis: disabling a container
doesn't stop its children being interactive anywhere else either.

Ctrl+Tab stays with ChoiceStrip, which uses it to switch tabs.

focusMoves now holds FocusMove rather than raw keycodes, so the direction is
decided in one place while the modifiers are still around, and a held key
repeats in the direction it was originally pressed with - the synthesized repeat
has no modifiers of its own. That also retires the keycode switch in
UpdateViewHierarchy and IsScrollKey, which had no other callers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SfY7iFJEjmRXf1XGrTs4MF

Common/UI/Root.cpp

@@ -1,3 +1,4 @@
+#include <algorithm>
 #include <deque>
 
 #include "ppsspp_config.h"
@@ -10,7 +11,7 @@
 
 namespace UI {
 
-static std::vector<int> focusMoves;
+static std::vector<FocusMove> 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<View *> 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);
 			}
 		}
 	}

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);
 

Common/UI/ViewGroup.cpp

@@ -489,6 +489,27 @@ NeighborResult ViewGroup::FindNeighbor(View *view, FocusMove direction, Neighbor
 	}
 }
 
+void ViewGroup::CollectTabOrder(std::vector<View *> *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<ViewGroup *>(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;

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<View *> *outViews) const;
+
 	bool CanBeFocused() const override { return false; }
 	bool IsViewGroup() const override { return true; }
 	bool ContainsSubview(const View *view) const override;

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<View *> 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[]) {