Regression: display of beam properties for a selected note

• Jan 16, 2019 - 17:49
Reported version
3.x-dev
Priority
P1 - High
Type
Graphical (UI)
Frequency
Few
Severity
S3 - Major
Reproducibility
Always
Status
closed
Regression
Yes
Workaround
No
Project

In version 2.x, when you selected a note and opened the beam properties palette, the current beam property of the note (e.g. "beam start", "auto beam", etc.) would be highlighted. This does not happen in version 3.


Comments

Status fixed active

Thanks for taking this on! It's somewhat of an improvement, but it only works for selection by clicking, not for any other means of selection (eg, cursor). So it ends up being more misleading than not showing anything, I'm afraid. I'd recommend going back to your original approach of doing it in MuseScore::endCmd(), and then keep the existing call you have only to catch the clickOffElement case.

Sorry I didn't have comments this detailed earlier, was hoping someone who knew this code better would do so.

In reply to by Marc Sabatella

You’re right! I actually had code to cover special bases which was why my function had the default parameter value of false, but for some reason when I did my final testing before the PR I ended up removing one of the function calls thinking it was unnecessary. Also by cursor, do you mean like using the left and right arrows? Thank you for the reminder, I will fix it when I get home or when I get the chance.

Great! Yes, I mean left/right arrow keys, there are various other similar commands to that can result in the selectin changing. As long as the selection is a single note or rest (easy to check, ask on telegram if you need help). the beam palette should be updated.

Status active PR created

https://github.com/musescore/MuseScore/pull/4766

Alright I fixed the issue and found another issue along the way that I fixed too. Upon deselection of the element where clickOffElement isn't set to true, I never updated the UI so it didn't show that no valid chordRest is selected until you hovered over the cells or changed the selection. I did have to do it in Musescore::endCmd() instead of Musescore::selectionChanged() based on my current knowledge though. I have a bit more explaining why in the pull request. Thank you for your help!

Fixed in branch master, commit 3472631b31

_Merge pull request #4766 from peterhieuvu/281957-beamproperties

fix #281957 move highlight update to work for cursor changes and on click of elements that aren't notes_

Fixed in branch 3.0.5, commit 2ef2bf50f1

_Merge pull request #4766 from peterhieuvu/281957-beamproperties

fix #281957 move highlight update to work for cursor changes and on click of elements that aren't notes_

Hmm, I was looking into the new QML code but it looks like it might be a bit more difficult to implement this time. The painting is a lot more abstracted away and there's no clear way it seems to mark something to be draw for selection based off of something else in the code.

This has been fixed with d1f1233, but there is still room for improvement. For one thing, the palette cell is deactivated when the user clicks on the canvas to drag the score, even though the selection does not change if the canvas is moved before the mouse is released. Now that ScoreView::mouseReleaseEvent() calls mscore->endCmd() after modifying the selection (like it should have done all along), it is no longer necessary to (mis)use the clickOffElement variable to determine whether or not the beam mode palette cells should be deactivated. Also, there is no reason to deactivate all palette cells if the selection contains multiple ChordRests, as long as all of the ChordRests have the same beam mode.

Status PR created fixed

Fixed in branch master, commit e3dd8fe885

_fix #281957: display of beam properties for a selected note

Do not deactivate the highlighted beam mode palette cell when the user clicks on the canvas to drag the score. Also, highlight the beam mode for multiple selected ChordRests, if they all have the same beam mode._

Fix version
3.4.0