Skip to content

FIX: Two clicks needed to move focus between parameter fields in the Input Actions editor [UUM-144339]#2439

Closed
Pauliusd01 wants to merge 1 commit into
developfrom
fix/uum-144339-nameandparameterslistview-rebuild
Closed

FIX: Two clicks needed to move focus between parameter fields in the Input Actions editor [UUM-144339]#2439
Pauliusd01 wants to merge 1 commit into
developfrom
fix/uum-144339-nameandparameterslistview-rebuild

Conversation

@Pauliusd01

@Pauliusd01 Pauliusd01 commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Description

Note

This pull request was generated automatically. Please review carefully before merging.

In the Input Actions editor's Action Properties panel, moving focus directly from one interaction/processor parameter field to another — for example a Normalize processor's Min to its Max — required two clicks instead of one. The first click was effectively swallowed and only the second moved focus.

The cause is in how a value commit is handled. When NameAndParametersListView.OnParametersChanged(ParameterListView listView, int index) commits an edited value via SerializedProperty.ApplyModifiedProperties(), StateContainer's TrackSerializedObjectValue callback (hardcoded to UIRebuildMode.Rebuild) turns that change into a full editor UI rebuild. The rebuild tears down and recreates the parameter field the click is moving focus into, so that first click lands on an element that no longer exists. The fix lets a value-only commit opt out of that single rebuild: StateContainer.IgnoreNextSerializedObjectChange() makes the next serialized-object change skip the rebuild (consumed one-shot), and OnParametersChanged calls it just before committing. The fields are left intact and focus moves on the first click. Structural edits (add/move/delete) deliberately do not opt out and still rebuild.

Testing status & QA

  • No automated test added — this is a UI Toolkit focus/rebuild-timing behaviour in the editor window that cannot be asserted reliably without a contrived, fragile multi-click focus simulation.
  • Manual QA: re-run the repro in UUM-144339 — add a processor or interaction with multiple parameters (e.g. Normalize), edit one field then click another, and confirm focus moves on the first click. Also confirm add/reorder/delete of interactions and processors, and switching selection between actions/bindings, still refresh correctly.

Overall Product Risks

  • Complexity: low
  • Halo Effect: low (one view method plus a one-shot opt-out flag on the shared StateContainer rebuild path; no public API or serialized data changes)
  • Risk rating: 3/5 — the value-commit path now opts out of the shared serialized-object-change rebuild via a one-shot flag, so a mis-scoped suppression could skip or defer an unrelated rebuild; the multi-click focus behaviour has no automated coverage.

Comments to reviewers

The suppression is a one-shot consumed by the next TrackSerializedObjectValue notification and is set only for value-only parameter commits (not add/move/delete). Worth confirming no other path depends on a rebuild firing for that specific commit.

Checklist

Before review:

  • Changelog entry added.
    • Explains the change in Changed, Fixed, Added sections.
    • For API change contains an example snippet and/or migration example.
    • JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
    • Jira port for the next release set as "Resolved".
  • Tests added/changed, if applicable.
    • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
    • Performance tests.
    • Integration tests.
  • Docs for new/changed API's.
    • Xmldoc cross references are set correctly.
    • Added explanation how the API works.
    • Usage code examples added.
    • The manual is updated, if needed.

During merge:

  • Commit message for squash-merge is prefixed with one of the list:
    • NEW: ___.
    • FIX: ___.
    • DOCS: ___.
    • CHANGE: ___.
    • RELEASE: 1.1.0-preview.3.

@Pauliusd01 Pauliusd01 self-assigned this Jun 16, 2026
@Pauliusd01

This comment was marked as outdated.

@u-pr

This comment was marked as outdated.

@Pauliusd01

This comment was marked as off-topic.

@u-pr

u-pr Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Test Plan

(Custom_review updated until commit 1a6cb31)

Updated: 2026-06-16 - Refined scenarios to include "no-change" blur logic verification and structural rebuild safety.

  • Test plan approved by PR author
  • Test execution in progress
  • Test execution complete
  • Ready for merge

Summary of Changes & Risk Assessment

Summary of Changes

This PR addresses a focus issue in the Input Actions editor where switching between parameter fields required two clicks. The fix introduces a one-shot m_IgnoreNextSerializedObjectChange flag in StateContainer to skip the UI rebuild when a parameter value is committed, preventing the VisualElement from being destroyed during the click interaction. It also includes a guard to ensure the flag is only armed when a value actually changes.

Risk Assessment

  • High Risk Areas: None identified.
  • Medium Risk Areas: Editor UI state consistency (ensuring the UI doesn't get "stuck" out of sync with serialized data).
  • Low Risk Areas: Input Action serialization, Processor/Interaction parameter parsing.

Test Scenarios

Functional Testing

  • Single Click Focus: Open the Input Actions editor. Add a Normalize processor. Click the Min field, type a value, then click directly into the Max field. Verify focus moves immediately and the Min value is saved.
  • Value Persistence: Modify multiple parameters (e.g., Scale, Clamp) in one session. Close and reopen the Input Actions window to ensure all ApplyModifiedProperties calls persisted the data correctly.
  • Interaction Parameters: Verify the same one-click focus behavior works for Interactions (e.g., Hold duration vs Tap time) within the NameAndParametersListView.

Regression Testing

  • Structural Rebuilds (Add/Delete): Add or remove a processor/interaction from the list. Verify that the UI does rebuild immediately. These actions should not trigger the "ignore" flag as they don't call OnParametersChanged.
  • Undo/Redo Support: Change a parameter value, then press Ctrl+Z. Verify that the UI correctly updates to show the previous value, confirming that the "ignore" flag doesn't interfere with standard Undo-triggered rebuilds.
  • Composite Bindings: Ensure parameters for composite bindings (e.g., 2D Vector) still behave correctly and save as expected.
🔍 Regression Deep Dive (additional risks identified)
  • Multi-selection Sync: Select multiple Actions. Modify a parameter in the properties pane. Verify that the change applies to all selected items (if supported) and focus behaves correctly.
  • Tab Navigation: Use the Tab key to move between parameters. Verify that tabbing out of a field still triggers a value commit without breaking the focus chain.

Edge Cases

  • Blur Without Change: Click into a parameter field but do not change the value, then click another field. Verify that NameAndParametersListView.cs:L85 correctly prevents arming the ignore flag.
  • Dangling Flag Check: After performing a "Blur Without Change," immediately perform a structural change (like deleting an action). Verify the UI still rebuilds, confirming no "dangling" ignore flag was left behind.
  • Rapid Switching: Rapidly click between different processors in the list while editing values to ensure no race conditions occur with the schedule.Execute logic.

💡 This test plan updates automatically when /test_plan is run on new commits. If you have any feedback, please reach out in #ai-qa


🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr

@Pauliusd01 Pauliusd01 force-pushed the fix/uum-144339-nameandparameterslistview-rebuild branch from d8ec951 to 769b139 Compare June 16, 2026 07:37
@Pauliusd01

This comment was marked as outdated.

@u-pr

This comment was marked as outdated.

…Input Actions editor [UUM-144339]

In the Input Actions editor, moving focus directly from one interaction/processor parameter field to another (for example a Normalize processor's Min to its Max) required two clicks instead of one.

Committing a parameter value calls SerializedProperty.ApplyModifiedProperties(), which StateContainer's TrackSerializedObjectValue callback turns into a full editor UI rebuild. That rebuild tore down and recreated the parameter field the click was moving focus into, so the first click was lost. NameAndParametersListView.OnParametersChanged now asks the StateContainer to skip the rebuild for that single value-only commit (StateContainer.IgnoreNextSerializedObjectChange), leaving the fields intact so focus moves on the first click. Structural edits (add/move/delete) do not opt out and still rebuild.

Jira: https://jira.unity3d.com/browse/UUM-144339
@Pauliusd01 Pauliusd01 force-pushed the fix/uum-144339-nameandparameterslistview-rebuild branch from 769b139 to 1a6cb31 Compare June 16, 2026 08:00
@Pauliusd01

This comment was marked as outdated.

@Pauliusd01

This comment was marked as off-topic.

@u-pr

This comment was marked as off-topic.

@u-pr

u-pr Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

This updated revision is fantastic and completely resolves the potential edge cases.

Key Improvements & Correctness

  1. Robust One-Shot Flag Lifecycle: By adding the comparison check if (m_ListProperty.stringValue == newValue) return;, you successfully prevent the m_IgnoreNextSerializedObjectChange flag from becoming armed and dangling on a focus-blur with no actual value edits. The flag is now strictly armed only when a genuine modification occurs, ensuring that standard UI rebuilds (such as those triggered by subsequent structural edits, window swaps, or Undo/Redo operations) are never accidentally swallowed.
  2. Redundant Write Prevention: Bailing early when the values are identical also optimizes editor performance by skipping redundant calls to ApplyModifiedProperties() and serialization overhead.
  3. Synchronous Safety: Because Unity's ApplyModifiedProperties() executes the property tracking bindings synchronously, the flag is armed and consumed immediately within the same call stack. This eliminates any deferred race conditions or asynchronous timing gaps.

The code is exceptionally clean, robust, and fully solves the double-click focus issue described in UUM-144339. This is ready to go.

📚 Additional Context
  • Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/StateContainer.cs
  • Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs

🤖 Helpful? 👍/👎

@Pauliusd01

Copy link
Copy Markdown
Collaborator Author

Seeing as this failed manual testing twice I am not happy with the risk. Only low risk fixes should be automated - closing

@Pauliusd01 Pauliusd01 closed this Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant