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: The "Add Control Scheme..." popup now requires explicit Save/Close user action (case ISXB-1131) #2044

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

AlexTyrer
Copy link
Collaborator

@AlexTyrer AlexTyrer commented Nov 6, 2024

Description

Previously clicking outside the popup window (anywhere in the editor) would close the popup window.

This would leave a new blank (unnamed, orphaned) control scheme populating the UI - this was confusing.

Now this popup will persist until the user explicitly chooses to Save or Cancel.

Testing status & QA

Manually testing the UI using the reproduction steps provided.
Local and automated testing.

Overall Product Risks

  • Complexity: minimal - it's single line change (deletion)
  • Halo Effect: minimal - just affects the behaviour of this popup window

Comments to reviewers

This issue manifested when the Input System started using the newer UIToolkit implementation of its UI.

It was a result of fixing another issue in this PR.

Discussions with @Pauliusd01 and @lyndon-unity led to this choice of behaviour change of this popup window:

  • it's the least surprising for the user
  • it avoids losing any unsaved changes without the user explicitly making a choice.

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.

After merge:

  • Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.

…t Save/Close user action (case ISXB-1131)

o Previously clicking outside the popup window (anywhere in the editor) would close the popup window.

o This would leave a new blank (unnamed, orphaned) control scheme populating the UI - this was confusing.

o Now this popup will persist until the user explictly chooses to Save or Cancel.
@AlexTyrer AlexTyrer changed the title [Input System] The "Add Control Scheme..." popup now requires explicit Save/Close user action (case ISXB-1131) FIX: The "Add Control Scheme..." popup now requires explicit Save/Close user action (case ISXB-1131) Nov 6, 2024
Copy link
Collaborator

@lyndon-unity lyndon-unity left a comment

Choose a reason for hiding this comment

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

Testing this on Alex's machine shows the issue is resolved, whist the other related issue is not regressed. It also seems a more intuitive UX flow.

@jfreire-unity
Copy link
Collaborator

Adding QA

Copy link
Collaborator

@Pauliusd01 Pauliusd01 left a comment

Choose a reason for hiding this comment

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

LGTM. Checked:

  • trying to interact with things within the asset while the control scheme popup is open (you can't, It's fully static. Would be great if it blinked when you click outside it just like on Windows, but I don't think that's a thing we can use in UITK)
  • entering play mode while it is open (it auto closes and refreshes the window, which I think is fine)
  • closing the input window while the popup is still there (same as the play mode scenario)
  • basic functionality of adding/removing/editing schemes

@jfreire-unity jfreire-unity merged commit 12ee89a into develop Nov 6, 2024
76 of 77 checks passed
@jfreire-unity jfreire-unity deleted the isxb-1131/add-control-scheme-popup-fix branch November 6, 2024 17:18
smnwttbr pushed a commit that referenced this pull request Nov 11, 2024
…se user action (case ISXB-1131) (#2044)

* [Input System] The "Add Control Scheme..." popup now requires explicit Save/Close user action (case ISXB-1131)

o Previously clicking outside the popup window (anywhere in the editor) would close the popup window.

o This would leave a new blank (unnamed, orphaned) control scheme populating the UI - this was confusing.

o Now this popup will persist until the user explictly chooses to Save or Cancel.

* [Input System] Added CHANGELOG entry for "Add Control Scheme..." popup fix (case ISXB-1131)

* [Input System] Fix typo in CHANGELOG entry - removed extraneous final closing parenthesis.

* [Input System] Licensing failure - try limiting to version 2021.3.45f
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.

4 participants