Skip to content

Fix #11325, #11363, refactor score movement handling code, and fix spelling and grammar #28138

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

godalming123
Copy link

@godalming123 godalming123 commented May 24, 2025

This PR is the successor to #28072.

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@godalming123 godalming123 changed the title Fix #11325 and refactor score movement handling code Fix #11325, #11363 and refactor score movement handling code May 24, 2025
@godalming123 godalming123 changed the title Fix #11325, #11363 and refactor score movement handling code Fix #11325, #11363, refactor score movement handling code, and fix spelling and grammar May 26, 2025
@godalming123
Copy link
Author

godalming123 commented May 26, 2025

I saw this yesterday:

The reason for the large padding was to enable zooming in, then out again to stay on a stable spot in the score.

This PR removes all of the padding, breaking this UX feature. So I started thinking about ways to remove the extra padding, or make the extra padding less intrusive while still keeping this UX feature. I came up with 2 ideas:

  1. Only allow the user to see the padding when they zoom in and out, while still constraining so that there is minimal padding the canvas when the user scrolls and pans around the canvas

  2. Move the mouse cursor so that it still hovers over the same part of the canvas even when that part is at a different position relative to the screen:

    2025-05-26.15-32-05.mp4

@Tantacrul what do you think of these 2 ideas?

@Tantacrul
Copy link
Contributor

Tantacrul commented May 27, 2025

It seems to zoom fine for me, although I'm just testing on trackpad.

The old version allowed too much space, which felt a bit silly, although I find the very tight spacing in this PR to be a little restrictive. It's very much a 'feeling' thing.

How about allowing a little more max space, like this?
image

@godalming123 godalming123 requested a review from cbjeukendrup May 28, 2025 11:12
@godalming123
Copy link
Author

@Tantacrul I've updated the code to allow 30% extra space when zooming in and out, but constrain the canvas to have a very small amount of extra space when panning. Is this good?

@Tantacrul
Copy link
Contributor

@godalming123 - as far as I can tell, it's exactly the same. There's far too tight a space around the score when zoomed in. Why do we want to do this? It feels restrictive for no purpose I can discern.

image

@godalming123
Copy link
Author

godalming123 commented May 28, 2025

@Tantacrul

As far as I can tell, it's exactly the same.

There should be extra space when you zoom in with your cursor outside the score:
https://github.com/user-attachments/assets/26a0cbff-5eef-4ac6-a013-bdb0396d82ec

Why do we want to do this? It feels restrictive for no purpose I can discern.

I don't want to have extra space when panning so that in the future I can add a feature where when there is no vertical scroll space and you use the scroll wheel, muse score can scroll horizontally without you having to press shift. This feature would be incredibly useful for continuous view.

@Tantacrul
Copy link
Contributor

There's no value in zooming on a blank space. It's just too tight.

I don't understand why it needs to be so tight or what value that brings. Just make it far less tight.

Also refactors score movement handling code, removes unused Limit scroll area to page borders checkbox, and fixes a spelling error and a grammar error
@cbjeukendrup
Copy link
Member

@Tantacrul Do you agree with the removal of the Limit scroll area to page borders preference from Preferences > Canvas?

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.

[MU4 Issue] Page View - Canvas size / white space [MU4 Issue] Continuous View - Whitespace
3 participants