Fix ISXB-1782: Invert processor (and other parameterless items) no lo…#2375
Fix ISXB-1782: Invert processor (and other parameterless items) no lo…#2375
Conversation
…nger show expandable foldout - When processor/interaction has no properties (hasUIToShow is false), do not add foldout content. - Disable foldout toggle and hide expand arrow via USS so row is label + buttons only. Made-with: Cursor
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
There was a problem hiding this comment.
This review identified several critical and medium priority issues in the NameAndParametersListView.cs file. The primary concerns involve a functional regression where list controls (Move/Delete) are disabled for certain items, potential memory leaks and performance degradation due to improper VisualElement management, and state persistence bugs caused by improper handling of recycled elements.
🤖 Helpful? 👍/👎
| { | ||
| // ISXB-1782: No expandable foldout when processor/interaction has no properties (e.g. Invert) | ||
| foldout.value = false; | ||
| foldout.SetEnabled(false); |
There was a problem hiding this comment.
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? 👍/👎
| if (parameterListView.hasUIToShow) | ||
| { | ||
| parameterListView.OnDrawVisualElements(foldout); | ||
| foldout.Add(new IMGUIContainer(parameterListView.OnGUI)); |
There was a problem hiding this comment.
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? 👍/👎
| parameterListView.OnDrawVisualElements(foldout); | ||
|
|
||
| foldout.Add(new IMGUIContainer(parameterListView.OnGUI)); | ||
| if (parameterListView.hasUIToShow) |
There was a problem hiding this comment.
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? 👍/👎
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #2375 +/- ##
========================================
Coverage 77.90% 77.91%
========================================
Files 476 481 +5
Lines 97613 97669 +56
========================================
+ Hits 76048 76096 +48
- Misses 21565 21573 +8 Flags with carried forward coverage won't be shown. Click here to find out more.
|
1 participant
Link: https://jira.unity3d.com/browse/ISXB-1783
Why it’s easy: Standard layout issue when the window is docked and narrow: top buttons (Save Asset, Devices, Control Schemes) overlap.
Fix: In the Input Actions Editor UI, add a minimum width and/or make the button bar wrap or use overflow (e.g. menu) when width is small so buttons don’t overlap.
Extra: Same editor as 1782; repro is “dock + resize to narrow”.
Made-with: Cursor
Description
Please fill this section with a description what the pull request is trying to address and what changes were made.
Testing status & QA
Please describe the testing already done by you and what testing you request/recommend QA to execute. If you used or created any testing project please link them here too for QA.
Overall Product Risks
Please rate the potential complexity and halo effect from low to high for the reviewers. Note down potential risks to specific Editor branches if any.
Comments to reviewers
Please describe any additional information such as what to focus on, or historical info for the reviewers.
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.