Skip to content

Fix ISXB-1782: Invert processor (and other parameterless items) no lo… #2375

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.

Sign up for GitHub

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

Open
K-Tone wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Fix ISXB-1782: Invert processor (and other parameterless items) no lo… #2375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ...utsystem/InputSystem/Editor/UITKAssetEditor/PackageResources/InputActionsEditorStyles.uss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@
.name-and-parameters-list-foldout {
}

/* ISXB-1782: Hide expand arrow when processor/interaction has no properties (e.g. Invert) */
.name-and-parameters-list-foldout--no-content .unity-foldout__checkmark {
display: none;
}

.name-and-parameters-list-view .name-and-parameters-list-foldout-button {
width: 12px;
height: 12px;
Expand Down
14 changes: 12 additions & 2 deletions ...m.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,19 @@

var foldout = container.Q<Foldout>("Foldout");
foldout.text = parameterListView.name;
parameterListView.OnDrawVisualElements(foldout);

foldout.Add(new IMGUIContainer(parameterListView.OnGUI));
if (parameterListView.hasUIToShow)
Copy link
Contributor

@u-pr u-pr bot Mar 12, 2026

Choose a reason for hiding this comment

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

medium

When using VisualElement recycling (common in virtualized lists like ListView), state changes such as SetEnabled and AddToClassList must be explicitly handled in both branches of the conditional logic. If a recycled element is reused for an item where hasUIToShow is true after being used for one where it was false, it will remain disabled and keep the --no-content class.

Consider resetting the state in the if block:

foldout.SetEnabled(true);
foldout.RemoveFromClassList("name-and-parameters-list-foldout--no-content");

🤖 Helpful? 👍/👎

{
parameterListView.OnDrawVisualElements(foldout);
foldout.Add(new IMGUIContainer(parameterListView.OnGUI));
Copy link
Contributor

@u-pr u-pr bot Mar 12, 2026

Choose a reason for hiding this comment

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

high

Adding a new IMGUIContainer (and potentially other elements via OnDrawVisualElements) without clearing the foldout first can lead to an accumulation of child elements if the VisualElement is recycled. This will result in memory leaks and degraded performance in the Editor as multiple containers stack on top of each other.

It is recommended to clear the content container before adding new elements, or to reuse an existing IMGUIContainer if one is already present in the hierarchy.

🤖 Helpful? 👍/👎

}
else
{

Check warning on line 180 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs#L180 

Added line #L180 was not covered by tests
// ISXB-1782: No expandable foldout when processor/interaction has no properties (e.g. Invert)
foldout.value = false;
foldout.SetEnabled(false);
Copy link
Contributor

@u-pr u-pr bot Mar 12, 2026

Choose a reason for hiding this comment

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

high

Calling foldout.SetEnabled(false) here disables the entire Foldout and implicitly all of its descendants.

Because the Move Up, Move Down, and Delete buttons are added as children to the foldout's header (container.Q<Toggle>()), this will disable those buttons as well. As a result, users will be completely unable to delete or reorder any processors or interactions that lack parameters (such as Invert). Additionally, disabling the element will visually gray out the processor's text, incorrectly making it look inactive.

Since you are already hiding the expand arrow via the .name-and-parameters-list-foldout--no-content USS class, and the foldout has no content to show, simply removing the SetEnabled(false) call should achieve the desired behavior without breaking the list controls.

🤖 Helpful? 👍/👎

foldout.AddToClassList("name-and-parameters-list-foldout--no-content");
}

Check warning on line 185 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs#L182-L185 

Added lines #L182 - L185 were not covered by tests
}
}
}
Expand Down
Loading