Previous to MS 2.2, it was possible to apply a break to a frame by selecting the frame then double-clicking a palette break symbol. This option seems to be missing in 2.2.
Also only system breaks on a measure act as toggles (when applied to measures via double-click), if I understand the intention of the code change, this is supposed to work for all breaks (but not for spacers).
In master you can apply any of the breaks or spacers to any of the frames at all currently.
Indeed, sorry for the regression. I am having build issues (again) and can't deal with this properly right now. Fix should be in cmdToggleLayoutBreak(). We would need to handle frames in the "else" clause of the selection().isRange() test. Easiest would be to test each el in the loop to see if it is a frame, and if so just call undoSetBreak right then and there, not bothering with the rest of code in that loop.
As for the other break types, I am not 100% sure the desired effect when adding a break of a given type if a break of another type is already present. In 2.1 effects seemed pretty inconsistent. Best to really think that through and do what's right.
There is some logic to that, but is it really what makes most sense with respect to usability? If there is a page break and I add a line break, I probably want it to replace the page break and vice versa, but not so with section breaks. So maybe they all toggle, but there is more to it in terms of effect on existing breaks?
Hmm, OK, page- and system breaks are mutually exclusive (when applied via drag'n'drop, it is possible to add a system break to an existing page break on a measure currently via double click), so adding one where the other exists should replace, but section breaks can be pure toggles as they can cooexist with page- and system breaks
OK, I finally have my build environment working again and can deal with this.
The issue with frames is easy enough to understand, shouldn't be a problem to fix.
The actual behavior in terms of what happens when applying breaks when breaks are already present is still worth looking at further. As far as I can tell, the behaviro in 2.2 is actually better in most respects with respect to what happens if breaks are already present, which is to say I don't think there is regression, but no doubt still room for further improvement. Here's what I see:
Single full measure selected, double-click palette icon for line or page break
* 2.1: adds selected break if not already present, replacing break of other type (section breaks are left alone)
* 2.2: for line break, toggles while leaving page break alone; for page break, adds only if no line break is present (same as Ctrl+Enter)
Frame selected, double-click palette icon for line or page break
* 2.1: adds selected break if not already present, replacing break of other type (also replaces section break)
* 2.2: nothing (the bug at hand)
Barline selected, Enter pressed
* 2.1: toggles line break, other breaks left alone
* 2.2: ditto
Barline selected, Ctrl+Enter pressed
* 2.1: adds page break only if no line break is present already
* 2.2: ditto
To me, while of course I want to fix the regression with respect to double-click not working at all on frames, pretty much nothing else about the 2.1 behavior described above is worth preserving. I think the ideal behavior for pretty much all of these cases is, toggle the specified line or page break, replace break of other type (leaving section breaks alone). For section breaks, a simple toggle leaving others alone.
That's what I'm planning on shooting for unless someone gives me a reason to do otherwise.
Oh, one other element to this: drag & drop. Currently behavior is same as 2.1: dropping a line break adds it (no toggle), replacing an existing page break. Dropping a page break does the converse.
Seems good to me, makes things work as I think we want. Unfortunately as mentioned breaks on frames are broken on master, so I can't test this code there. But in principle the same code should work, just update the static_casts.
Ultimately it would be nice to also factor out "do it" code here from the the "figure out what to do it to" code, then re-use the "do it" code for Measure::drop() and Box::drop() as well (for that matter, move the handling to MeasureBase::drop()). But I'll let someone else worry about that once master is in better shape.
The versions 2.1 and 2.2 are for users who create the issues to specify their MuseScore version. But 2.3-dev version is used to specify that the issue is in work for 2.3 now and may be available in nightlies.
Comments
Confirmed.
It's a pity: broken on last March 9/10.
- This nightly works: 3413e07
- Not this one : ab27065
So, the most likely I guess it's a side effect of (same last mentioned nightly/commit) : https://github.com/musescore/MuseScore/pull/3528/files
To fix: #232931: Make Toggle System Break shortcut available when note or measure selected
Or ? https://github.com/musescore/MuseScore/commit/bf602c194394924b589ea9312…
Confirmed
Agree on the title, but disagree about minor priority. It is (was) a useful, and expected behaviour.
so do I ;-)
Also only system breaks on a measure act as toggles (when applied to measures via double-click), if I understand the intention of the code change, this is supposed to work for all breaks (but not for spacers).
In master you can apply any of the breaks or spacers to any of the frames at all currently.
Indeed, sorry for the regression. I am having build issues (again) and can't deal with this properly right now. Fix should be in cmdToggleLayoutBreak(). We would need to handle frames in the "else" clause of the selection().isRange() test. Easiest would be to test each el in the loop to see if it is a frame, and if so just call undoSetBreak right then and there, not bothering with the rest of code in that loop.
As for the other break types, I am not 100% sure the desired effect when adding a break of a given type if a break of another type is already present. In 2.1 effects seemed pretty inconsistent. Best to really think that through and do what's right.
In reply to Indeed, sorry for the… by Marc Sabatella
Each break type should toggle itself, e.g. adding a page break if there is one already should remove it.
There is some logic to that, but is it really what makes most sense with respect to usability? If there is a page break and I add a line break, I probably want it to replace the page break and vice versa, but not so with section breaks. So maybe they all toggle, but there is more to it in terms of effect on existing breaks?
Hmm, OK, page- and system breaks are mutually exclusive (when applied via drag'n'drop, it is possible to add a system break to an existing page break on a measure currently via double click), so adding one where the other exists should replace, but section breaks can be pure toggles as they can cooexist with page- and system breaks
OK, I finally have my build environment working again and can deal with this.
The issue with frames is easy enough to understand, shouldn't be a problem to fix.
The actual behavior in terms of what happens when applying breaks when breaks are already present is still worth looking at further. As far as I can tell, the behaviro in 2.2 is actually better in most respects with respect to what happens if breaks are already present, which is to say I don't think there is regression, but no doubt still room for further improvement. Here's what I see:
Single full measure selected, double-click palette icon for line or page break
* 2.1: adds selected break if not already present, replacing break of other type (section breaks are left alone)
* 2.2: for line break, toggles while leaving page break alone; for page break, adds only if no line break is present (same as Ctrl+Enter)
Frame selected, double-click palette icon for line or page break
* 2.1: adds selected break if not already present, replacing break of other type (also replaces section break)
* 2.2: nothing (the bug at hand)
Barline selected, Enter pressed
* 2.1: toggles line break, other breaks left alone
* 2.2: ditto
Barline selected, Ctrl+Enter pressed
* 2.1: adds page break only if no line break is present already
* 2.2: ditto
To me, while of course I want to fix the regression with respect to double-click not working at all on frames, pretty much nothing else about the 2.1 behavior described above is worth preserving. I think the ideal behavior for pretty much all of these cases is, toggle the specified line or page break, replace break of other type (leaving section breaks alone). For section breaks, a simple toggle leaving others alone.
That's what I'm planning on shooting for unless someone gives me a reason to do otherwise.
Oh, one other element to this: drag & drop. Currently behavior is same as 2.1: dropping a line break adds it (no toggle), replacing an existing page break. Dropping a page break does the converse.
Here's a PR for 2.3:
https://github.com/musescore/MuseScore/pull/3634
Seems good to me, makes things work as I think we want. Unfortunately as mentioned breaks on frames are broken on master, so I can't test this code there. But in principle the same code should work, just update the static_casts.
Ultimately it would be nice to also factor out "do it" code here from the the "figure out what to do it to" code, then re-use the "do it" code for Measure::drop() and Box::drop() as well (for that matter, move the handling to MeasureBase::drop()). But I'll let someone else worry about that once master is in better shape.
Not a regression in 2.3, but in 2.2, also the PR is still pending review
The versions 2.1 and 2.2 are for users who create the issues to specify their MuseScore version. But 2.3-dev version is used to specify that the issue is in work for 2.3 now and may be available in nightlies.
Not quite what we used to muse that field for... as discussed on IRC, use tags for that
Fixed in branch 2.3, commit f9e89a28c4
fix #271450: double-click to set breaks on frames
Fixed in branch 2.3, commit 532506d347
Merge pull request #3634 from MarcSabatella/271450-toggle-breaks
fix #271450: double-click to set breaks on frames
Fixed in branch master, commit db751e93ea
fix #271450: double-click to set breaks on frames
Automatically closed -- issue fixed for 2 weeks with no activity.