Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Arrow key navigation of Input Actions after Action rename. #2082

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
64 changes: 63 additions & 1 deletion Assets/Tests/InputSystem.Editor/InputActionsEditorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ public override void OneTimeSetUp()
{
base.OneTimeSetUp();
m_Asset = AssetDatabaseUtils.CreateAsset<InputActionAsset>();
m_Asset.AddActionMap("First Name");
var actionMap = m_Asset.AddActionMap("First Name");
m_Asset.AddActionMap("Second Name");
m_Asset.AddActionMap("Third Name");

actionMap.AddAction("Action One");
actionMap.AddAction("Action Two");
}

public override void OneTimeTearDown()
Expand Down Expand Up @@ -55,6 +58,19 @@ IEnumerator WaitForActionMapRename(int index, bool isActive, double timeoutSecs
}, $"WaitForActionMapRename {index} {isActive}", timeoutSecs);
}

IEnumerator WaitForActionRename(int index, bool isActive, double timeoutSecs = 5.0)
{
return WaitUntil(() =>
{
var actionItems = m_Window.rootVisualElement.Q("actions-container").Query<InputActionsTreeViewItem>().ToList();
if (actionItems.Count > index && actionItems[index].IsFocused == isActive)
{
return true;
}
return false;
}, $"WaitForActionRename {index} {isActive}", timeoutSecs);
}

#endregion

[Test]
Expand Down Expand Up @@ -173,5 +189,51 @@ public IEnumerator CanDeleteActionMap()
Assert.That(m_Window.currentAssetInEditor.actionMaps[0].name, Is.EqualTo("First Name"));
Assert.That(m_Window.currentAssetInEditor.actionMaps[1].name, Is.EqualTo("Third Name"));
}

[UnityTest]
public IEnumerator CanRenameAction()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome you've added a proper editor test for this. Thank you!

{
var actionContainer = m_Window.rootVisualElement.Q("actions-container");
var actionItem = actionContainer.Query<InputActionsTreeViewItem>().ToList();
Assume.That(actionItem[1].Q<Label>("name").text, Is.EqualTo("Action Two"));

m_Window.rootVisualElement.Q<TreeView>("actions-tree-view").Focus();
m_Window.rootVisualElement.Q<TreeView>("actions-tree-view").selectedIndex = 1;

// Selection change triggers a state change, wait for the scheduler to process the frame
yield return WaitForSchedulerLoop();
yield return WaitForNotDirty();
yield return WaitForFocus(m_Window.rootVisualElement.Q<TreeView>("actions-tree-view"));

// Re-fetch the actions since the UI may have refreshed.
actionItem = actionContainer.Query<InputActionsTreeViewItem>().ToList();

// Click twice to start the rename
SimulateClickOn(actionItem[1]);
// If the item is already focused, don't click again
if (!actionItem[1].IsFocused)
{
SimulateClickOn(actionItem[1]);
}

yield return WaitForActionRename(1, isActive: true);

// Rename the action
SimulateTypingText("New Name");

// Wait for rename to end
yield return WaitForActionRename(1, isActive: false);

// Check on the UI side
actionContainer = m_Window.rootVisualElement.Q("actions-container");
Assume.That(actionContainer, Is.Not.Null);
actionItem = actionContainer.Query<InputActionsTreeViewItem>().ToList();
Assert.That(actionItem, Is.Not.Null);
Assert.That(actionItem.Count, Is.EqualTo(2));
Assert.That(actionItem[1].Q<Label>("name").text, Is.EqualTo("New Name"));

// Check on the asset side
Assert.That(m_Window.currentAssetInEditor.actionMaps[0].actions[1].name, Is.EqualTo("New Name"));
}
}
#endif
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ however, it has to be formatted properly to pass verification tests.
- Fixed documentation to clarify bindings with modifiers `overrideModifiersNeedToBePressedFirst` configuration [ISXB-806](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-806).
- Fixed an issue in `Samples/Visualizers/GamepadVisualizer.unity` sample where the visualization wouldn't handle device disconnects or current device changes properly (ISXB-1243).
- Fixed an issue when displaying Serialized InputAction's Processor properties inside the Inspector window. [ISXB-1269](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1269)
- Fixed arrow key navigation of Input Actions after Action rename. [ISXB-1024](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1024)

### Changed
- Added back the InputManager to InputSystem project-wide asset migration code with performance improvement (ISX-2086).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,13 @@ public override void RedrawUI(ViewState viewState)
{
m_ActionsTreeView.Clear();
m_ActionsTreeView.SetRootItems(viewState.treeViewData);
// UI toolkit doesn't behave the same on 6000.0 way when refreshing items
// On previous versions, we need to call Rebuild() to refresh the items since refreshItems() is less predicatable
#if UNITY_6000_0_OR_NEWER
m_ActionsTreeView.RefreshItems();
#else
m_ActionsTreeView.Rebuild();
#endif
if (viewState.newElementID != -1)
{
m_ActionsTreeView.SetSelectionById(viewState.newElementID);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ internal class InputActionsTreeViewItem : VisualElement
private const string kRenameTextField = "rename-text-field";
public event EventCallback<string> EditTextFinished;

// for testing purposes to know if the item is focused to accept input
internal bool IsFocused { get; private set; } = false;

private bool m_IsEditing;
private static InputActionsTreeViewItem s_EditingItem = null;

Expand All @@ -34,9 +37,13 @@ public InputActionsTreeViewItem()
renameTextfield.selectAllOnFocus = true;
renameTextfield.selectAllOnMouseUp = false;


RegisterCallback<MouseDownEvent>(OnMouseDownEventForRename);
renameTextfield.RegisterCallback<FocusOutEvent>(e => OnEditTextFinished());
renameTextfield.RegisterCallback<FocusInEvent>(e => IsFocused = true);
renameTextfield.RegisterCallback<FocusOutEvent>(e =>
{
OnEditTextFinished();
IsFocused = false;
});
}

public Label label => this.Q<Label>();
Expand Down
Loading