Skip to content
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

[MU4 Issue] Apply current palette element command not implemented #13281

Closed
MarcSabatella opened this issue Sep 10, 2022 · 7 comments · Fixed by #18652
Closed

[MU4 Issue] Apply current palette element command not implemented #13281

MarcSabatella opened this issue Sep 10, 2022 · 7 comments · Fixed by #18652
Assignees
Labels
community Issues particularly suitable for community contributors to work on P2 Priority: Medium

Comments

@MarcSabatella
Copy link
Contributor

MarcSabatella commented Sep 10, 2022

Describe the bug
MuseScore 3 had a command "apply current palette element" to easily reapply the most recently-used palette element. This was a great usability feature and especially valuable for blind users even though it had no default shortcut. This command is missing in MU4.

To Reproduce
Steps to reproduce the behavior:

  1. Go to Edit / Preferences / Shortcuts
  2. Search for "palette"
  3. See error: no "Apply current palette element" command available

Expected behavior
The "apply current palette element" should be available, and ideally, should have a default shortcut assigned even though there was none in MU3. FWIW, I normally used "\".

Platform information

  • OS: Linux

Additional context
I know there is some new functionality coming for shortcuts via the GSoC project, but on the assumption that none of the pending work for that is being targeted for 4.0, re-implementing this one command would be a big help for now.

There are a handful of other commands that were dropped from MU3, some consciously, some perhaps less so. This to me is the one most worth adding back for 4.0.

@wizofaus
Copy link
Contributor

You normally used ""?
There is still "applyCurrentPaletteElement" in PalettesPanel.qml but nothing calling it...hard to judge whether it will actually work.

@MarcSabatella
Copy link
Contributor Author

Sorry, I typed a backslash but GitHub ate it. Fixed now.

@MarcSabatella
Copy link
Contributor Author

@Tantacrul - also not beta-critical at all, but, for me personally, I think I miss the command more than any other single MU3 feature. An unbelievably useful productivity tool.

@Tantacrul Tantacrul added the P2 Priority: Medium label Oct 13, 2022
@Tantacrul Tantacrul added the community Issues particularly suitable for community contributors to work on label Oct 13, 2022
@Tantacrul Tantacrul added this to To do in 4.x SHORTLIST via automation Oct 13, 2022
@Tantacrul Tantacrul added this to To do in Keyboard navigation and shortcuts via automation Oct 13, 2022
@MarcSabatella
Copy link
Contributor Author

MarcSabatella commented Oct 14, 2022

I can make an attempt, if I can get a little help. I knew how things worked in MU3 but it still is a lot of trial and error for me in MU4.

@wizofaus - I assume you're talking about the code here:

https://github.com/musescore/MuseScore/blob/master/src/palette/qml/MuseScore/Palette/PalettesPanel.qml#L46-L47

It looks like the actual work then is done here:

https://github.com/musescore/MuseScore/blob/master/src/palette/qml/MuseScore/Palette/internal/PaletteTree.qml#L111-L120

which looks about right to me (not that I know my way QML at all). So, it seems promising.

In theory, then, all we'd need to do is make sure the command exists and calls the applyCurrentPaletteElement function. The place where the command would be to be defined, I assume, is here:

https://github.com/MarcSabatella/MuseScore/blob/apply-current-palette-element/src/palette/internal/paletteuiactions.cpp#L33-L71

And then after defining the command itself there, I'd need to register it here:

https://github.com/musescore/MuseScore/blob/master/src/notation/internal/notationactioncontroller.cpp

That last part is the mystery to me - I have no idea what magic it would take to actually reference the applyCurrentPaletteElement function here.

@cbjeukendrup
Copy link
Contributor

@MarcSabatella You could use the "palette-search" action as an example.

The class PaletteRootModel is instantiated in QML, and registers itself as a handler for this action. When the action is called, the model emits a signal, namely paletteSearchRequested().

PaletteRootModel::PaletteRootModel(QObject* parent)
: QObject(parent)
{
dispatcher()->reg(this, "palette-search", [this]() {
emit paletteSearchRequested();
});
}

In QML, there is a handler for this signal; in this handler, the relevant function in QML is called.

PaletteRootModel {
id: paletteRootModel
onPaletteSearchRequested: {
palettesPanelHeader.startSearch()
}
}

So, you can basically just copy & paste the handler registrations and fill in the correct names, action code and QML function.

@MarcSabatella
Copy link
Contributor Author

Great, that gets me started! Signals, yeah, makes sense. I'll give it a shot.

BTW, the palette search command is actually defined in the palette code but I had been thinking it made sense for the apply current palette element command to be defined in notation, since that's where the focus would be on executing it and where action would actually take effect. But maybe I'll try it all here in the palette code first.

@wizofaus
Copy link
Contributor

I'd expect it to be in the Palette code - I wouldn't think the Notation code should depend on/ know about the Palette generally.

worldwideweary added a commit to worldwideweary/MuseScore that referenced this issue Jul 17, 2023
and allow [enter] to apply first result while in palette-search area
worldwideweary added a commit to worldwideweary/MuseScore that referenced this issue Jul 18, 2023
and allow [enter] to apply first result while in palette-search area
worldwideweary added a commit to worldwideweary/MuseScore that referenced this issue Jul 18, 2023
and allow [enter] to apply first result while in palette-search area
worldwideweary added a commit to worldwideweary/MuseScore that referenced this issue Jul 18, 2023
and allow [enter] to apply first result while in palette-search area
worldwideweary added a commit to worldwideweary/MuseScore that referenced this issue Jul 18, 2023
and allow [enter] to apply first result while in palette-search area
worldwideweary added a commit to worldwideweary/MuseScore that referenced this issue Jul 24, 2023
and allow [enter] to apply first result while in palette-search area.

Also update highlighting of palette cell when applying/clicking.
@cbjeukendrup cbjeukendrup removed this from To do in 4.x SHORTLIST Jul 25, 2023
@cbjeukendrup cbjeukendrup moved this from To do to On test in Keyboard navigation and shortcuts Jul 25, 2023
Keyboard navigation and shortcuts automation moved this from On test to Done Jul 26, 2023
cbjeukendrup added a commit that referenced this issue Jul 26, 2023
…lement-command

Fix #13281: Implement [Apply current palette element] command
Nekotizimo pushed a commit to Nekotizimo/MuseScore that referenced this issue Jul 27, 2023
and allow [enter] to apply first result while in palette-search area.

Also update highlighting of palette cell when applying/clicking.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Issues particularly suitable for community contributors to work on P2 Priority: Medium
5 participants