Skip to content

Fix offset spinboxes not updated - #34868

Merged
mathesoncalum merged 1 commit into
musescore:mainfrom
SilverGreen93:mihai_offset_spinboxes
Sep 17, 2026
Merged

mathesoncalum merged 1 commit into
musescore:mainfrom
SilverGreen93:mihai_offset_spinboxes

Conversation

@SilverGreen93

Copy link
Copy Markdown
Contributor

Resolves: #34867

GeneralSettingsModel::onNotationChanged(), the hook that fires on every score/property change never forwards to AppearanceSettingsModel, so the Offset fields only ever refreshed when driven from their own widget.

Fixed by forwarding the notification to both m_appearanceSettingsModel and m_playbackProxyModel, matching the pattern already used in onCurrentNotationChanged() just below it.
This makes the Offset spinboxes update after dragging text on the canvas.

  • I signed the CLA as username:
  • 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.
  • I understand all aspects of the code I'm contributing and I'm able to explain it if requested.
  • The code compiles and runs on my machine, preferably after each commit individually. I have manually tested and verified that my changes fulfil their intended purpose.
  • No prior attempts to resolve this problem exist, or if they do, I listed them in my PR description and described how I avoided repeating past mistakes.
  • There are no unnecessary changes.
  • I created a unit test or vtest to verify the changes I made (if applicable).

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: a5980083-c04c-4eba-8aac-efea44c23bc2

📥 Commits

Reviewing files that changed from the base of the PR and between 109b1fe and 59fe570.

📒 Files selected for processing (2)
  • src/propertiespanel/qml/MuseScore/PropertiesPanel/general/appearance/appearancesettingsmodel.h
  • src/propertiespanel/qml/MuseScore/PropertiesPanel/general/generalsettingsmodel.cpp

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

GeneralSettingsModel::onNotationChanged now forwards property and style changes to AppearanceSettingsModel and PlaybackProxyModel. AppearanceSettingsModel::onNotationChanged is publicly accessible so the parent model can invoke it. This updates appearance controls when notation properties change, including alignment offsets modified by dragging.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 59fe5

Appearance offset controls now receive notation-change updates after text is dragged, keeping their displayed coordinates current. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: fixing offset spinboxes that do not update.
Description check ✅ Passed The description explains the defect, root cause, fix, affected behavior, and testing status. It includes the required template sections. The unit-test item is unchecked, but no test is clearly require…
Linked Issues check ✅ Passed The changes address issue [#34867]. GeneralSettingsModel::onNotationChanged() now forwards notifications to AppearanceSettingsModel, allowing the offset spinboxes to refresh after text movement. The p…
Out of Scope Changes check ✅ Passed The changes are limited to notification forwarding and the required access adjustment. Both changes support the linked issue and stated pull request objective.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped musescore/muse_framework.git.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mathesoncalum mathesoncalum left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good - thanks!

I signed the CLA as username:

For future reference this should point to your username on MuseScore.org (I know you've signed it before, but just thought I'd mention).

@zacjansheski

Copy link
Copy Markdown
Contributor

Tested on MacOS Tahoe 26.2, Windows 11, Ubuntu 22.04.3. Approved
#34867 FIXED

@mathesoncalum
mathesoncalum merged commit 0765eb0 into musescore:main Sep 17, 2026
15 checks passed
@SilverGreen93

Copy link
Copy Markdown
Contributor Author

Looks good - thanks!

I signed the CLA as username:

For future reference this should point to your username on MuseScore.org (I know you've signed it before, but just thought I'd mention).

Sorry, I missed that part, as in other project I contribute to I only need to check that I signed. Will fill in my username in the future.
Thanks!

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.

Alignment offset spinboxes are not updated

4 participants