Regression: can't apply Breaks to frames via double-click on palette break symbol

• Apr 16, 2018 - 10:53
Reported version
2.3
Type
Functional
Severity
S4 - Minor
Status
closed
Project
Tags

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.


Comments

Title Breaks and spacers: missing command in 2.2? Inability to apply a break by double-clicking on frames
Severity S4 - Minor

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…

Title Inability to apply a break by double-clicking on frames Regression: can't apply Breaks and spacers to frames via double-click on palette break symbol
Severity S4 - Minor  

Confirmed

Title Regression: can't apply Breaks and spacers to frames via double-click on palette break symbol Inability to apply a break to frames by double-clicking
Severity S4 - Minor
Title Inability to apply a break to frames by double-clicking Regression: can't apply Breaks and spacers to frames via double-click on palette break symbol

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.

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.

Status (old) active patch (code needs review)
Status active  

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.

Status (old) active patch (code needs review)
Status active  
Reported version 2.3 2.2

Not a regression in 2.3, but in 2.2, also the PR is still pending review

Status
Reported version 2.2 2.3

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.

Title Regression: can't apply Breaks and spacers to frames via double-click on palette break symbol Regression: can't apply Breaks to frames via double-click on palette break symbol