InputSystem icon indicating copy to clipboard operation
InputSystem copied to clipboard

ISX-1851 Remove title from editor window

Open ekcoh opened this issue 1 year ago • 11 comments

Description

Removes the header title from UITK editor window when used outside Project Settings. Improves styling of header title in Project Settings.

Changes made

Removes the header title from editor window as called out in user feedback here: https://forum.unity.com/threads/project-wide-actions-1-8-0-pre-2-release-feedback-wanted-new-update-released.1488843/ by Walter_hulsebos.

Modifies header title styling to look more similar to other Settings. Not perfect but closer. Also alignes menu button with container window menu button.

Screenshot 2024-01-29 at 12 29 47

Screenshot 2024-01-29 at 12 16 04

Notes

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.
    • FogBugz ticket attached, example ([case %number%](https://issuetracker.unity3d.com/issues/...)).
    • FogBugz is marked as "Resolved" with next release version correctly set.
  • [ ] 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.

ekcoh avatar Jan 29 '24 11:01 ekcoh

Reverted the change to title I did while checking it, also replaced incorrect screenshot

ekcoh avatar Jan 29 '24 11:01 ekcoh

Just needs formatting running on the commit, but looks good

Should hopefully be fixed now.

ekcoh avatar Jan 29 '24 12:01 ekcoh

Getting 2 different errors that don't repro on develop when adding new maps to fresh assets. Steps to repro: Create a new input asset -> make sure it is in auto save and add a new map -> get UITK error from Editor side -> delete that map and add one again -> get nullref from Input System side

https://github.com/Unity-Technologies/InputSystem/assets/54306142/041b2118-8abf-4413-ad48-0597fc3b195f

Pauliusd01 avatar Jan 29 '24 13:01 Pauliusd01

updating status

@Pauliusd01 - Checked the following

  1. I merged latest changes from develop into this branch just to be sure its up-to-date.
  2. I tried the steps described on 2022.3.18f1 and I cannot replicate doing exactly the same as in the video.
  3. I tried the steps described on 2023.2.b04 and I cannot replicate doing exactly the same as in the video.

I noticed the exception happens in Unity native UI framework code. Do you happen to have a full stack trace. A video is great for reproduction but not very helpful to find the problem when I cannot replicate. Are you saying this exception only happens on this branch and not on develop using the same Unity version?

ekcoh avatar Jan 30 '24 13:01 ekcoh

For clarity, as discussed outside this PR, I failed to reproduce on macOS while @Pauliusd01 got the errors on Windows 10 so there is likely a platform-dependent issue, likely in UITK implementation and indirectly triggered by this PR.

ekcoh avatar Jan 31 '24 14:01 ekcoh

@Pauliusd01 I pushed a potential fix for the renaming problem that attempted to enter rename mode without a selection as I was able to reproduce the reported issue on Win10 but not on macOS so something regarding renaming is platform dependent. Feel free to retest.

ekcoh avatar Feb 01 '24 13:02 ekcoh

I dinged further into this yesterday and the problematic behavior is caused by renaming logic where there isn't a clean adoption of the model-view design in the view components. Hence there is work needed on renaming and selection logic (unrelated to changes on this branch).

ekcoh avatar Feb 02 '24 12:02 ekcoh

I think the code in this PR supersedes this https://github.com/Unity-Technologies/InputSystem/pull/1834

lyndon-unity avatar Feb 09 '24 10:02 lyndon-unity

I've moved all key changes over into the other PR so this one shouldn't land.

@lyndon-unity What exactly was moved then? Does this imply the regression introduced by this branch and attempted to be solved in sub-branch https://github.com/Unity-Technologies/InputSystem/pull/1833/ was also ported into https://github.com/Unity-Technologies/InputSystem/pull/1834? I believe it would be good to avoid porting that regression into 1834.

ekcoh avatar Feb 12 '24 21:02 ekcoh

I've moved all key changes over into the other PR so this one shouldn't land.

@lyndon-unity What exactly was moved then? Does this imply the regression introduced by this branch and attempted to be solved in sub-branch #1833 was also ported into #1834? I believe it would be good to avoid porting that regression into 1834.

I haven't looked at the sub branch and the 'regression'. The header changes in this PR are applied. I did apply RenameNewActionMaps() too so if thats the regression area then the sub branch parts will need applying. (https://github.com/Unity-Technologies/InputSystem/pull/1834/files#diff-2627872bea2748f8f1725742226018473e44f270300bd2cca84b782223b0057c )

lyndon-unity avatar Feb 12 '24 23:02 lyndon-unity

@lyndon-unity the regression seems completely unrelated to the changes on this PR but for some reason these changes triggers a problem that seems to only be visible on Windows and not macOS. Likely related to focus/selection, hence the changes on the other branch. The RenameNewActionMaps changes where added as a way to mitigate the problem. The problem surfaced just by changing the UXML which was the odd part.

ekcoh avatar Feb 13 '24 08:02 ekcoh

I believe this PR is obsolete now due to other changes. Will close this PR but leave branch existing for a while longer if there is need to pick any cherries from it.

ekcoh avatar Feb 29 '24 21:02 ekcoh