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

Support scaling around center when scaling with select box #29949

Merged
merged 5 commits into from
Sep 28, 2024

Conversation

minetoblend
Copy link
Contributor

@minetoblend minetoblend commented Sep 21, 2024

Will scale around the center when pressing alt while scaling things with the select box.

Had to move the event handling for toggling the distance snap grid to (Osu/Catch)HitObjectComposer so the event could be prevented by the scale handle, but since OsuHitObjectComposer already handles the grid snap toggle this should hopefully be fine.

2024-09-21.14-18-19.mp4
2024-09-21.14-23-38.mp4

@minetoblend minetoblend changed the title Scale around selection center while pressing alt when scaling using select box Support scaling around center when scaling with select box Sep 21, 2024
Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

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

Because this is yet another key modifier specific interaction, I'd love to see test coverage for this. Probably the sort that also covers the pre-existing Shift behaviour. And probably a test that covers using both Shift and this simultaneously.

@@ -50,14 +50,14 @@ protected override void OnDrag(DragEvent e)

rawScale = convertDragEventToScaleMultiplier(e);

applyScale(shouldLockAspectRatio: isCornerAnchor(originalAnchor) && e.ShiftPressed);
applyScale(shouldLockAspectRatio: isCornerAnchor(originalAnchor) && e.ShiftPressed, ignoreAnchor: e.AltPressed);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Alt is not the conventional choice for this behaviour. In gimp this is under Ctrl. And I believe it's the same in photoshop (although I don't have a copy to check right now).

I'd probably prefer this was Ctrl. Is there a particular reason for choosing Alt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used Alt because from my experience it is the typical modifier key used for this (photoshop, figma, indesign, krita all use the Alt key), but after some research I found some programs that use Ctrl too (affinity photo, microsoft word), so I'm open to change it. Alt is definitely the first key I tried out when I was originally looking for this feature though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh figma does use Alt... interesting.

I did test https://www.photopea.com/ which is a sort of photoshop clone and that used Ctrl so I extrapolated photoshop did the same, although possibly too rashly.

I'm not sure I'm hugely fussed as to which one it is in the end, but it'd be good to know if one is more frequently used than the other...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can compile a list of what modifier is used for different programs, however I'm a bit unsure which programs to check here since the original list already includes pretty much every program that I could think of off the top of my head (apart from checking the entire adobe/affinity suite I guess).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nah it's fine. I'm ok with just going with alt for now. If figma and adobe use that then that's probably the one users will most expect.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 24, 2024
@bdach bdach requested a review from peppy September 27, 2024 08:11
@peppy peppy merged commit c46d787 into ppy:master Sep 28, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants