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
66 changes: 65 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 @@ -177,5 +193,53 @@ 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]
[Ignore("Instability, see ISXB-1284")]
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 and focus to return from text field
yield return WaitForSchedulerLoop();
yield return WaitForFocus(m_Window.rootVisualElement.Q<TreeView>("actions-tree-view"));

// 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 @@ -33,6 +33,7 @@ however, it has to be formatted properly to pass verification tests.
- 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 an issue with default device selection when adding new Control Scheme.
- Fixed an issue where action map delegates were not updated when the asset already assigned to the PlayerInput component were changed [ISXB-711](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-711).
- 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 All @@ -51,28 +58,38 @@ public void UnregisterInputField()
renameTextfield.UnregisterCallback<FocusOutEvent>(e => OnEditTextFinished());
}

private float lastSingleClick;
private double lastSingleClick;
private static InputActionsTreeViewItem selected;

private void OnMouseDownEventForRename(MouseDownEvent e)
{
if (e.clickCount != 1 || e.button != (int)MouseButton.LeftMouse || e.target == null)
return;

if (selected == this && Time.time - lastSingleClick < 3f)
var now = EditorApplication.timeSinceStartup;
if (selected == this && now - lastSingleClick < 3)
{
FocusOnRenameTextField();
e.StopImmediatePropagation();
lastSingleClick = 0;
return;
}
lastSingleClick = Time.time;
lastSingleClick = now;
selected = this;
}

public void Reset()
{
if (m_IsEditing)
{
lastSingleClick = 0;
delegatesFocus = false;

renameTextfield.AddToClassList(InputActionsEditorConstants.HiddenStyleClassName);
label.RemoveFromClassList(InputActionsEditorConstants.HiddenStyleClassName);
s_EditingItem = null;
m_IsEditing = false;
}
EditTextFinished = null;
m_IsEditing = false;
}

public void FocusOnRenameTextField()
Expand All @@ -87,7 +104,7 @@ public void FocusOnRenameTextField()

//a bit hacky - e.StopImmediatePropagation() for events does not work like expected on ListViewItems or TreeViewItems because
//the listView/treeView reclaims the focus - this is a workaround with less overhead than rewriting the events
DelayCall();
schedule.Execute(() => renameTextfield.Q<TextField>().Focus()).StartingIn(120);
renameTextfield.SelectAll();

s_EditingItem = this;
Expand All @@ -99,12 +116,6 @@ public static void CancelRename()
s_EditingItem?.OnEditTextFinished();
}

async void DelayCall()
{
await Task.Delay(120);
renameTextfield.Q<TextField>().Focus();
}

private void OnEditTextFinished()
{
if (!m_IsEditing)
Expand All @@ -123,6 +134,7 @@ private void OnEditTextFinished()
renameTextfield.schedule.Execute(() => renameTextfield.SetValueWithoutNotify(text));
return;
}

EditTextFinished?.Invoke(text);
}
}
Expand Down
Loading