Ready for Review - [MouseJump] move "common" classes into separate project #34333

mikeclayton · 2024-08-17T19:31:26Z

Summary of the Pull Request

This is an attempt to break PR #33486 ([Mouse Jump] - Customisable appearance - borders, margins, colours) down into smaller chunks because it's gotten unmanageable due to its age and number of commits (currently 37 commits / 205 files changed and 25 merge conflicts).

This PR just moves the files in the "MouseJumpUI\Common" folder into a separate *.csproj project so that it can eventually be referenced by the SettingsUI project to draw a sample preview image when style settings are changed. There's no functionality changes in the code, just a move of the existing classes into a separate project (and updating the namespaces), so it'll hopefully be a lot easier to review than #33486, which can eventually be cancelled.

A separate follow-up PR will re-implement the Settings UI changes to close PR #33486 and Issue #27511.

PR Checklist

Detailed Description of the Pull Request / Additional comments

This PR moves all of the code currently held in the MouseJumpUI project under "src\modules\MouseUtils\MouseJumpUI\Common" into a new project "MouseJump.Common". This will allow the project to be referenced from the SettingsUI project in a follow-up PR as part of the work to implement customisable style settings.

Validation Steps Performed

Workflow tests
- Automated tests passing locally
- Minimal actions workflow (spelling check) passing for PR
- Full actions workflow (msbuild) passing for PR
UI tests
- Happy path
  - preview image appears when activated
  - clicking the preview image moves the mouse cursor to the correct location
  - right-click dismisses the preview image without moving the mouse
  - pressing escape dismisses the preview image without moving the mouse
- Works on multiple monitors with different dpi scaling settings (e.g. 100% vs 150%)
  - [Mouse Jump] - new feature - #23216 #23566 (comment)
  - [Mouse Jump] - new feature - #23216 #23566 (comment)
- Handling negative coordinates on non-primary monitors if higher or "lefter" than primary monitor
  - [Mouse Jump] - new feature - #23216 #23566 (comment)
- Mouse crosshair moves when Mouse Jump moves the cursor (mouse clicks and keyboard shortcuts)
  - Mouse pointer Crosshair & Mouse Jump Issue #24523
  - [MouseJump]Also simulate mouse input event #24527
- Activating when the preview window is already visible moves the form to the new mouse position
- Number and key shortcuts (1-9, Home / End, Left / Right Arrow) jump to the appropriate monitor
- Number-pad shortcuts (1-9 jump to the appropriate monitor
Lifecycle tests
- Starting PowerToys Runner launches MouseJumpUI.exe when enabled, and not when disabled
- Enabling / disabling Mouse Jump in settings starts / stops MouseJumpUI.exe
- Exiting PowerToys Runner stops MouseJumpUI.exe
- Killing runner exe via Task Manager stops MouseJumpUI.exe
- Stopping Visual Studio local debug run stops MouseJumpUI.exe
  - note - runner needs to be in non-admin mode otherwise Visual Studio debugger disconnects at launch
- Hotkey and size settings are automatically reloaded when config file is modified from Settings UI
- ~~Hotkey and size settings are automatically reloaded when config file is modified manually (e.g. in notepad) while runner and MouseJumpUI.exe are running~~
Internal Test Suite
- Enable Mouse Jump. Then:
- Press the activation shortcut and verify the screens preview appears.
- Change activation shortcut and verify that new shortcut triggers Mouse Jump.
- Click around the screen preview and ensure that mouse cursor jumped to clicked location.
- Reorder screens in Display settings and confirm that Mouse Jump reflects the change and still works correctly.
- Change scaling of screens and confirm that Mouse Jump still works correctly.
- Unplug additional monitors and confirm that Mouse Jump still works correctly.
- Disable Mouse Jump and verify that the module is not activated when you press the activation shortcut.

…soft#25482

… the checks # Conflicts: # src/modules/MouseUtils/MouseJump.Common.UnitTests/MouseJump.Common.UnitTests.csproj

# Conflicts: # src/modules/MouseUtils/MouseJump.Common.UnitTests/MouseJump.Common.UnitTests.csproj

…25482

microsoft#25482

mikeclayton · 2024-08-22T21:52:15Z

So it looks like adding <UseWinforms>true</UseWinForms> to MouseJump.Common.csproj has fixed the problem with verifyDepsJsonLibraryVersions.ps1 detecting a different version of System.Drawing.Common.dll being referenced in MouseJump.Common versus all the other modules.

I'm not keen on it as a fix as MouseJump.Common only really needs a reference to System.Drawing.Common and not the whole WinForms, but I can't work out how to get the build to use the right version of System.Drawing.Common, so this might have to do for now :-S...

# Conflicts: # PowerToys.sln

mikeclayton · 2024-08-23T13:38:01Z

I think this PR is ready for review now - the manual and automated tests all work and the build pipeline is green.

The only remaining piece of work is to incorporate the new "MouseJump.Common.dll" assembly into the installer. I'm not super-confident about what is required for that - would someone from the core team be able to help?

mikeclayton · 2024-09-26T19:51:32Z

Fixed a bunch of merge conflicts - build is working again.

mikeclayton · 2024-10-18T09:20:13Z

@jaimecbernardo - just a quick head-up - before this gets merged into main the new MouseJump.Common.dll needs to be added to the installer, which I wasn't confident about doing myself...

jaimecbernardo · 2024-10-18T09:47:40Z

@jaimecbernardo - just a quick head-up - before this gets merged into main the new MouseJump.Common.dll needs to be added to the installer, which I wasn't confident about doing myself...

Thanks for the heads up. It should be added automatically, but it might need to be included in signing.

jaimecbernardo

Hi @mikeclayton , I've given it a try but after building in Visual Studio the MouseJump exe doesn't seem to be created. Can you please try building the PR from a clean state and verifying? I think the changes done to the .csproj might be related to this issue.
Thank you for the contribution, but can you please make the PR testable? 😉 Thank you!

mikeclayton · 2024-10-18T10:13:12Z

Hi @mikeclayton , I've given it a try but after building in Visual Studio the MouseJump exe doesn't seem to be created. Can you please try building the PR from a clean state and verifying? I think the changes done to the .csproj might be related to this issue. Thank you for the contribution, but can you please make the PR testable? 😉 Thank you!

Oh, interesting. I'll take a look tonight (UK time)...

mikeclayton · 2024-10-18T11:34:19Z

Looks like I removed these from the MouseJumpUI.csproj and MouseJump.Common.csproj somewhere along the line...

    <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
    <AppendRuntimeIdentifierToOutputPath>false</AppendRuntimeIdentifierToOutputPath>

so it was building in the wrong folder.

I'll re-add them later...

mikeclayton · 2024-10-18T23:59:51Z

@jaimecbernardo - the latest commit appears to put the build output in the right location now when I run it locally.

jaimecbernardo · 2024-10-21T12:58:17Z

@jaimecbernardo - the latest commit appears to put the build output in the right location now when I run it locally.

I can confirm it's working well now. Thank you :) Going to spin some builds to verify signing and do the final review after.

jaimecbernardo

LGTM! Thank you for the contribution!

Clayton added 2 commits August 17, 2024 20:00

[Mouse Jump] - moving common code to MouseJump.Common project - micro…

ba8b7cf

…soft#25482

[Mouse Jump] - fixing warnings in MouseJump.Common - microsoft#25482

2fc1901

mikeclayton marked this pull request as draft August 17, 2024 19:31

Clayton added 5 commits August 17, 2024 21:19

[MouseJump] - cherrypick 5653b4 - Exclude MouseJump Common tests from…

35690c6

… the checks # Conflicts: # src/modules/MouseUtils/MouseJump.Common.UnitTests/MouseJump.Common.UnitTests.csproj

[mOuSEjUMP] - cherry pick 61aab9 - Fix ci build issues

8275ced

# Conflicts: # src/modules/MouseUtils/MouseJump.Common.UnitTests/MouseJump.Common.UnitTests.csproj

[Mouse Jump] - remove project type guids - microsoft#25482

f3b4a21

[Mouse Jump] - simplify mousejump *.csproj files - microsoft#25482

7fcc9ab

[Mouse Jump] - fixing broken tests - microsoft#25482

7ca44c7

This comment has been minimized.

Sign in to view

[Mouse Jump] - fixing broken build - microsoft#25482

76c83f2

This comment has been minimized.

Sign in to view

Clayton added 2 commits August 22, 2024 21:55

[Mouse Jump] - editing MouseJump.Common.UnitTests.csproj - microsoft#…

997dc13

…25482

[Mouse Jump] - editing MouseJump.Common.csproj (UseWindowsForms=true) -

e9345d6

microsoft#25482

This comment has been minimized.

Sign in to view

[Mouse Jump] - fixing spellcheck - microsoft#25482

797a49b

Merge branch 'main' into origin/dev/mikeclayton/mousejump-common

3c755d9

# Conflicts: # PowerToys.sln

mikeclayton marked this pull request as ready for review August 23, 2024 13:36

mikeclayton requested review from crutkas, DHowett and ethanfangg as code owners August 23, 2024 13:36

mikeclayton changed the title ~~WIP - [MouseJump] move "common" classes into separate project~~ Ready for Review - [MouseJump] move "common" classes into separate project Aug 23, 2024

crutkas added the Needs-Review This Pull Request awaits the review of a maintainer. label Aug 26, 2024

crutkas added the In for .86 label Sep 26, 2024

[MouseJump] - fix merge conflicts from main - microsoft#25482

7288763

mikeclayton and others added 2 commits September 29, 2024 11:00

[MouseJump] - enabled implicit usings - microsoft#25482

ccfb027

Merge branch 'main' into origin/dev/mikeclayton/mousejump-common

491765e

jaimecbernardo requested changes Oct 18, 2024

View reviewed changes

[Mouse Jump] - re-add csproj attributes - microsoft#27511

25689c8

ci: Fix signing of Common dll

2b5abe8

crutkas approved these changes Oct 21, 2024

View reviewed changes

jaimecbernardo approved these changes Oct 21, 2024

View reviewed changes

jaimecbernardo merged commit dc7c7ef into microsoft:main Oct 21, 2024
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ready for Review - [MouseJump] move "common" classes into separate project #34333

Ready for Review - [MouseJump] move "common" classes into separate project #34333

mikeclayton commented Aug 17, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mikeclayton commented Aug 22, 2024 •

edited

Loading

mikeclayton commented Aug 23, 2024 •

edited

Loading

mikeclayton commented Sep 26, 2024

mikeclayton commented Oct 18, 2024

jaimecbernardo commented Oct 18, 2024

jaimecbernardo left a comment

mikeclayton commented Oct 18, 2024

mikeclayton commented Oct 18, 2024

mikeclayton commented Oct 18, 2024

jaimecbernardo commented Oct 21, 2024

jaimecbernardo left a comment

Ready for Review - [MouseJump] move "common" classes into separate project #34333

Ready for Review - [MouseJump] move "common" classes into separate project #34333

Conversation

mikeclayton commented Aug 17, 2024 • edited Loading

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mikeclayton commented Aug 22, 2024 • edited Loading

mikeclayton commented Aug 23, 2024 • edited Loading

mikeclayton commented Sep 26, 2024

mikeclayton commented Oct 18, 2024

jaimecbernardo commented Oct 18, 2024

jaimecbernardo left a comment

Choose a reason for hiding this comment

mikeclayton commented Oct 18, 2024

mikeclayton commented Oct 18, 2024

mikeclayton commented Oct 18, 2024

jaimecbernardo commented Oct 21, 2024

jaimecbernardo left a comment

Choose a reason for hiding this comment

mikeclayton commented Aug 17, 2024 •

edited

Loading

mikeclayton commented Aug 22, 2024 •

edited

Loading

mikeclayton commented Aug 23, 2024 •

edited

Loading