Home Explore Help
Sign In
cpp
/
RmlUi
mirror of https://github.com/mikke89/RmlUi.git
2
0
Fork 0
Files Issues 0 Wiki
Browse Source

Fix some issues with tabbing focus.

- Tabbing now works when a tabable element is a direct child of the body.
- Tabbing backward from the document now focuses on the last tabable element.
- Added unit test for verification.
Michael Ragazzon 4 years ago
parent
02d37b3b9a
commit
9ed2af527d
5 changed files with 265 additions and 29 deletions
Unified View Show Diff Stats
  1. 26 27
      Source/Core/ElementDocument.cpp
  2. 1 1
      Tests/Data/VisualTests/issue_143_overflow_in_absolutely_positioned_input.rml
  3. 235 0
      Tests/Source/UnitTests/ElementDocument.cpp
  4. 1 1
      Tests/Source/VisualTests/main.cpp
  5. 2 0
      changelog.md

+ 26 - 27
Source/Core/ElementDocument.cpp
View File 

@@ -553,13 +553,15 @@ void ElementDocument::OnResize()
 
 enum class CanFocus { Yes, No, NoAndNoChildren };
 
 enum class CanFocus { Yes, No, NoAndNoChildren };
 
 static CanFocus CanFocusElement(Element* element)
 
 static CanFocus CanFocusElement(Element* element)
 
 {
 
 {
 
-	if (element->IsPseudoClassSet("disabled"))
 
+	if (!element->IsVisible())
 
 		return CanFocus::NoAndNoChildren;
 
 		return CanFocus::NoAndNoChildren;
 
 
 
-	if (!element->IsVisible())
 
+	const ComputedValues& computed = element->GetComputedValues();
 
+
 
+	if (computed.focus == Style::Focus::None)
 
 		return CanFocus::NoAndNoChildren;
 
 		return CanFocus::NoAndNoChildren;
 
 
 
-	if (element->GetComputedValues().tab_index == Style::TabIndex::Auto)
 
+	if (computed.tab_index == Style::TabIndex::Auto)
 
 		return CanFocus::Yes;
 
 		return CanFocus::Yes;
 
 
 
 	return CanFocus::No;
 
 	return CanFocus::No;
 
@@ -573,7 +575,7 @@ static CanFocus CanFocusElement(Element* element)
 
 // anticlock wise direction depending if you're searching forward or backward respectively
 
 // anticlock wise direction depending if you're searching forward or backward respectively
 
 Element* ElementDocument::FindNextTabElement(Element* current_element, bool forward)
 
 Element* ElementDocument::FindNextTabElement(Element* current_element, bool forward)
 
 {
 
 {
 
-	// If we're searching forward, check the immediate children of this node first off
 
+	// If we're searching forward, check the immediate children of this node first off.
 
 	if (forward)
 
 	if (forward)
 
 	{
 
 	{
 
 		for (int i = 0; i < current_element->GetNumChildren(); i++)
 
 		for (int i = 0; i < current_element->GetNumChildren(); i++)
 
@@ -582,23 +584,19 @@ Element* ElementDocument::FindNextTabElement(Element* current_element, bool forw
 
 	}
 
 	}
 
 
 
 	// Now walk up the tree, testing either the bottom or top
 
 	// Now walk up the tree, testing either the bottom or top
 
-	// of the tree, depending on whether we're going forwards
 
-	// or backwards respectively
 
-	//
 
-	// If we make it all the way up to the document, then
 
-	// we search the entire tree (to loop back round)
 
+	// of the tree, depending on whether we're going forward
 
+	// or backward respectively.
 
 	bool search_enabled = false;
 
 	bool search_enabled = false;
 
 	Element* document = current_element->GetOwnerDocument();
 
 	Element* document = current_element->GetOwnerDocument();
 
 	Element* child = current_element;
 
 	Element* child = current_element;
 
 	Element* parent = current_element->GetParentNode();
 
 	Element* parent = current_element->GetParentNode();
 
 	while (child != document)
 
 	while (child != document)
 
 	{
 
 	{
 
-		for (int i = 0; i < parent->GetNumChildren(); i++)
 
+		const int num_children = parent->GetNumChildren();
 
+		for (int i = 0; i < num_children; i++)
 
 		{
 
 		{
 
 			// Calculate index into children
 
 			// Calculate index into children
 
-			int child_index = i;
 
-			if (!forward)
 
-				child_index = parent->GetNumChildren() - i - 1;
 
+			const int child_index = forward ? i : (num_children - i - 1);
 
 			Element* search_child = parent->GetChild(child_index);
 
 			Element* search_child = parent->GetChild(child_index);
 
 
 
 			// Do a search if its enabled
 
 			// Do a search if its enabled
 
@@ -614,21 +612,22 @@ Element* ElementDocument::FindNextTabElement(Element* current_element, bool forw
 
 		// Advance up the tree
 
 		// Advance up the tree
 
 		child = parent;
 
 		child = parent;
 
 		parent = parent->GetParentNode();
 
 		parent = parent->GetParentNode();
 
+		search_enabled = false;
 
+	}
 
 
 
-		if (parent == document)
 
-		{
 
-			// When we hit the top, see if we can focus the document first.
 
-			if (CanFocusElement(document) == CanFocus::Yes)
 
-				return document;
 
-			
-			// Otherwise, search the entire tree to loop back around.
 
-			search_enabled = true;
 
-		}
 
-		else
 
-		{
 
-			// Prepare for the next iteration by disabling searching.
 
-			search_enabled = false;
 
-		}
 
+	// We could not find anything to focus along this direction.
 
+
 
+	// If we can focus the document, then focus that now.
 
+	if (current_element != document && CanFocusElement(document) == CanFocus::Yes)
 
+		return document;
 
+
 
+	// Otherwise, search the entire document tree. This way we will wrap around.
 
+	const int num_children = document->GetNumChildren();
 
+	for (int i = 0; i < num_children; i++)
 
+	{
 
+		const int child_index = forward ? i : (num_children - i - 1);
 
+		if (Element* result = SearchFocusSubtree(document->GetChild(child_index), forward))
 
+			return result;
 
 	}
 
 	}
 
 
 
 	return nullptr;
 
 	return nullptr;

+ 1 - 1
Tests/Data/VisualTests/issue_143_overflow_in_absolutely_positioned_input.rml
View File 

@@ -26,7 +26,7 @@
 
 
 
 <body>
 
 <body>
 
 	<input type="text" value="a bunch of text that's longer than the input" />
 
 	<input type="text" value="a bunch of text that's longer than the input" />
 
-	<br />
 
+	<br/><br/>
 
 	<div class="hover">hover here</div>
 
 	<div class="hover">hover here</div>
 
 </body>
 
 </body>
 
 </rml>
 
 </rml>

+ 235 - 0
Tests/Source/UnitTests/ElementDocument.cpp
View File 

@@ -0,0 +1,235 @@
 
+/*
 
+ * This source file is part of RmlUi, the HTML/CSS Interface Middleware
 
+ *
 
+ * For the latest information, see http://github.com/mikke89/RmlUi
 
+ *
 
+ * Copyright (c) 2008-2010 CodePoint Ltd, Shift Technology Ltd
 
+ * Copyright (c) 2019 The RmlUi Team, and contributors
 
+ *
 
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
 
+ * of this software and associated documentation files (the "Software"), to deal
 
+ * in the Software without restriction, including without limitation the rights
 
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 
+ * copies of the Software, and to permit persons to whom the Software is
 
+ * furnished to do so, subject to the following conditions:
 
+ *
 
+ * The above copyright notice and this permission notice shall be included in
 
+ * all copies or substantial portions of the Software.
 
+ *
 
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
 
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 
+ * THE SOFTWARE.
 
+ *
 
+ */
 
+
 
+#include "../Common/TestsShell.h"
 
+#include <RmlUi/Core/Context.h>
 
+#include <RmlUi/Core/Element.h>
 
+#include <RmlUi/Core/ElementDocument.h>
 
+#include <doctest.h>
 
+#include <algorithm>
 
+
 
+using namespace Rml;
 
+
 
+static const String document_focus_rml = R"(
 
+<rml>
 
+<head>
 
+	<link type="text/rcss" href="/assets/rml.rcss"/>
 
+	<link type="text/rcss" href="/assets/invader.rcss"/>
 
+	<style>
 
+		button {
 
+			display: inline-block;
 
+			tab-index: auto;
 
+		}
 
+		:focus {
 
+			image-color: #af0;
 
+		}
 
+		:disabled {
 
+			image-color: #666;
 
+		}
 
+		.hide {
 
+			visibility: hidden;
 
+		}
 
+		.nodisplay {
 
+			display: none;
 
+		}
 
+	</style>
 
+</head>
 
+
 
+<body id="body">
 
+<input type="checkbox" id="p1"/> P1
 
+<label><input type="checkbox" id="p2"/> P2</label>
 
+<p>
 
+	<input type="checkbox" id="p3"/><label for="p3"> P3</label>
 
+</p>
 
+<p class="nodisplay">
 
+	<input type="checkbox" id="p4"/><label for="p4"> P4</label>
 
+</p>
 
+<input type="checkbox" id="p5"/> P5
 
+<p>
 
+	<input type="checkbox" id="p6" disabled/><label for="p6"> P6</label>
 
+</p>
 
+<div>
 
+	<label><input type="checkbox" id="p7"/> P7</label>
 
+	<label class="hide"><input type="checkbox" id="p8"/> P8</label>
 
+	<label><button id="p9"> P9</button></label>
 
+	<label><input type="checkbox" id="p10"/> P10</label>
 
+</div>
 
+<div id="container">
 
+	<p> Deeply nested
 
+		<span>
 
+			<input type="checkbox" id="p11"/> P11
 
+		</span>
 
+		<input type="checkbox" id="p12"/> P12
 
+	</p>
 
+	<input type="checkbox" id="p13"/> P13
 
+</div>
 
+</body>
 
+</rml>
 
+)";
 
+
 
+static const String focus_forward = "p1 p2 p3 p5 p7 p9 p10 p11 p12 p13";
 
+
 
+
 
+TEST_CASE("elementdocument.focus")
 
+{
 
+	Context* context = TestsShell::GetContext();
 
+	REQUIRE(context);
 
+
 
+	ElementDocument* document = context->LoadDocumentFromMemory(document_focus_rml);
 
+	REQUIRE(document);
 
+	document->Show();
 
+
 
+	context->Update();
 
+	context->Render();
 
+
 
+	TestsShell::RenderLoop();
 
+
 
+	document->Focus();
 
+
 
+	StringList ids;
 
+	StringUtilities::ExpandString(ids, focus_forward, ' ');
 
+
 
+	REQUIRE(!ids.empty());
 
+
 
+	// Forward direction
 
+	for(const String& id : ids)
 
+	{
 
+		context->ProcessKeyDown(Input::KI_TAB, 0);
 
+		CHECK(context->GetFocusElement()->GetId() == id);
 
+	}
 
+
 
+	// Wrap around
 
+	context->ProcessKeyDown(Input::KI_TAB, 0);
 
+	CHECK(context->GetFocusElement()->GetId() == ids[0]);
 
+
 
+	document->Focus();
 
+
 
+	std::reverse(ids.begin(), ids.end());
 
+
 
+	// Reverse direction
 
+	for (const String& id : ids)
 
+	{
 
+		context->ProcessKeyDown(Input::KI_TAB, Input::KM_SHIFT);
 
+		CHECK(context->GetFocusElement()->GetId() == id);
 
+	}
 
+
 
+	// Wrap around (reverse)
 
+	context->ProcessKeyDown(Input::KI_TAB, Input::KM_SHIFT);
 
+	CHECK(context->GetFocusElement()->GetId() == ids[0]);
 
+
 
+	// Tab to document
 
+	{
 
+		document->SetProperty("tab-index", "auto");
 
+		document->UpdateDocument();
 
+
 
+		context->ProcessKeyDown(Input::KI_TAB, 0);
 
+		CHECK(context->GetFocusElement()->GetId() == "body");
 
+	}
 
+
 
+	// Tab from container element
 
+	{
 
+		Element* container = document->GetElementById("container");
 
+		REQUIRE(container);
 
+
 
+		container->Focus();
 
+		context->ProcessKeyDown(Input::KI_TAB, 0);
 
+		CHECK(context->GetFocusElement()->GetId() == "p11");
 
+
 
+		container->Focus();
 
+		context->ProcessKeyDown(Input::KI_TAB, Input::KM_SHIFT);
 
+		CHECK(context->GetFocusElement()->GetId() == "p10");
 
+	}
 
+
 
+	// Single element
 
+	{
 
+		document->SetProperty("tab-index", "none");
 
+		document->SetInnerRML(R"(<input type="checkbox" id="p1"/> P1)");
 
+		document->UpdateDocument();
 
+
 
+		document->Focus();
 
+		context->ProcessKeyDown(Input::KI_TAB, 0);
 
+		CHECK(context->GetFocusElement()->GetId() == "p1");
 
+		context->ProcessKeyDown(Input::KI_TAB, 0);
 
+		CHECK(context->GetFocusElement()->GetId() == "p1");
 
+		context->ProcessKeyDown(Input::KI_TAB, Input::KM_SHIFT);
 
+		CHECK(context->GetFocusElement()->GetId() == "p1");
 
+
 
+		document->SetProperty("tab-index", "auto");
 
+		document->UpdateDocument();
 
+
 
+		document->Focus();
 
+		context->ProcessKeyDown(Input::KI_TAB, 0);
 
+		CHECK(context->GetFocusElement()->GetId() == "p1");
 
+		context->ProcessKeyDown(Input::KI_TAB, 0);
 
+		CHECK(context->GetFocusElement()->GetId() == "body");
 
+		context->ProcessKeyDown(Input::KI_TAB, Input::KM_SHIFT);
 
+		CHECK(context->GetFocusElement()->GetId() == "p1");
 
+		context->ProcessKeyDown(Input::KI_TAB, Input::KM_SHIFT);
 
+		CHECK(context->GetFocusElement()->GetId() == "body");
 
+	}
 
+
 
+	// Single, non-tabable element
 
+	{
 
+		document->SetProperty("tab-index", "none");
 
+		document->SetInnerRML(R"(<div id="child"/>)");
 
+		document->UpdateDocument();
 
+		Element* child = document->GetChild(0);
 
+
 
+		document->Focus();
 
+		context->ProcessKeyDown(Input::KI_TAB, 0);
 
+		CHECK(context->GetFocusElement()->GetId() == "body");
 
+		context->ProcessKeyDown(Input::KI_TAB, Input::KM_SHIFT);
 
+		CHECK(context->GetFocusElement()->GetId() == "body");
 
+
 
+		child->Focus();
 
+		context->ProcessKeyDown(Input::KI_TAB, 0);
 
+		CHECK(context->GetFocusElement()->GetId() == "child");
 
+		context->ProcessKeyDown(Input::KI_TAB, Input::KM_SHIFT);
 
+		CHECK(context->GetFocusElement()->GetId() == "child");
 
+
 
+		document->SetProperty("tab-index", "auto");
 
+		document->UpdateDocument();
 
+
 
+		document->Focus();
 
+		context->ProcessKeyDown(Input::KI_TAB, 0);
 
+		CHECK(context->GetFocusElement()->GetId() == "body");
 
+		context->ProcessKeyDown(Input::KI_TAB, Input::KM_SHIFT);
 
+		CHECK(context->GetFocusElement()->GetId() == "body");
 
+
 
+		child->Focus();
 
+		context->ProcessKeyDown(Input::KI_TAB, 0);
 
+		CHECK(context->GetFocusElement()->GetId() == "body");
 
+		child->Focus();
 
+		context->ProcessKeyDown(Input::KI_TAB, Input::KM_SHIFT);
 
+		CHECK(context->GetFocusElement()->GetId() == "body");
 
+	}
 
+
 
+	document->Close();
 
+
 
+	TestsShell::ShutdownShell();
 
+}

+ 1 - 1
Tests/Source/VisualTests/main.cpp
View File 

@@ -84,7 +84,7 @@ int main(int RMLUI_UNUSED_PARAMETER(argc), char** RMLUI_UNUSED_PARAMETER(argv))
 
 
 
 	// Generic OS initialisation, creates a window and attaches OpenGL.
 
 	// Generic OS initialisation, creates a window and attaches OpenGL.
 
 	if (!Shell::Initialise() ||
 
 	if (!Shell::Initialise() ||
 
-		!Shell::OpenWindow("Load Document Sample", shell_renderer, window_width, window_height, true))
 
+		!Shell::OpenWindow("Visual tests", shell_renderer, window_width, window_height, true))
 
 	{
 
 	{
 
 		Shell::Shutdown();
 
 		Shell::Shutdown();
 
 		return -1;
 
 		return -1;

+ 2 - 0
changelog.md
View File 

@@ -169,6 +169,8 @@ Improved Lua plugin in several aspects.
 
 - Fix \<textarea\> getting an unnecessary horizontal scrollbar. [#122](https://github.com/mikke89/RmlUi/issues/122)
 
 - Fix \<textarea\> getting an unnecessary horizontal scrollbar. [#122](https://github.com/mikke89/RmlUi/issues/122)
 
 - Fix text position changing in input fields when selecting text and font has kerning.
 
 - Fix text position changing in input fields when selecting text and font has kerning.
 
 - Fix text-decoration not always being regenerated. [#119](https://github.com/mikke89/RmlUi/issues/119)
 
 - Fix text-decoration not always being regenerated. [#119](https://github.com/mikke89/RmlUi/issues/119)
 
+- Fix tabbing navigation when tabable elements are direct children of body.
 
+- Fix tabbing navigation in reverse direction from body.
 
 
 
 ### Deprecated functionality
 
 ### Deprecated functionality