Palette shortcut #3252

divya-urs · 2017-07-30T19:09:59Z

This pull request adds the ability to assign a shortcut to a palette element.
A new dialog (Palette Shortcut Manager) has been added which displays a list of palette elements in the current workspace along with the shortcut assigned to each element. A shortcut can be defined or cleared using the palette shortcut manager. The palette shortcut manager can be accessed from the preferences dialog, or by using a shortcut command.
A shortcut can also be assigned using the palette cell context menu or a shortcut command which adds a shortcut to the currently selected palette element.
Shortcuts are saved, so a shortcut defined in a session can be used in later sessions.

Jojo-Schmitz · 2017-07-30T20:25:03Z

mscore/musescore.cpp

+            }
+      for (int i = 0; i < preferences.paletteCellList.size(); ++i) {
+            PaletteCellDescription* d = &preferences.paletteCellList[i];
+                  registerPaletteShortcut(d);


This is a work in progress. Please refrain to nitpick for now, since the code will change quite a lot.

Jojo-Schmitz · 2017-07-30T20:26:48Z

mscore/palette.cpp

@@ -1462,6 +1559,7 @@ void Palette::read(XmlReader& e)
                                                add = false; // action is not valid, don't add it to the palette.
                                                }
                                          }
+


Jojo-Schmitz · 2017-08-22T16:43:14Z

@divya-urs: your last commit broke it, it doesn't compile anymore

divya-urs · 2017-08-22T18:23:37Z

Sorry, I'll fix it

…

On 22 Aug 2017 10:13 p.m., "Joachim Schmitz" ***@***.***> wrote: @divya-urs <https://github.com/divya-urs>: your last commit broke it, it doesn't compile anymore — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3252 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AXlfLdysok_UDVqDBTCRdOn0wyvrd6tTks5sawUwgaJpZM4OnuBy> .

Jojo-Schmitz · 2017-08-23T07:46:47Z

@divya-urs: Please check (compiler) warnings introduced by your code: https://travis-ci.org/musescore/MuseScore/jobs/267297595#L2315-L2319 and https://travis-ci.org/musescore/MuseScore/jobs/267297595#L2503-L2511 and https://travis-ci.org/musescore/MuseScore/jobs/267297595#L2702-L2705

divya-urs · 2017-08-23T07:50:34Z

Okay, I will check.

Jojo-Schmitz · 2017-08-23T19:37:05Z

Looks like this added further warnings.

divya-urs · 2017-08-24T03:03:29Z

I was fixing a bug, so I had introduced a few unused variables. I'll remove them in my next commit.

…

On 24 Aug 2017 1:07 a.m., "Joachim Schmitz" ***@***.***> wrote: Looks like this added further warnings. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3252 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AXlfLQnuHAkck-6UA9aO_Xqf8rcuPtCWks5sbH9mgaJpZM4OnuBy> .

Jojo-Schmitz · 2017-08-24T05:47:21Z

Ah, so I just missunderstood what that commit was about.

MarcSabatella · 2017-08-25T16:46:00Z

mscore/menus.cpp

@@ -1526,7 +1526,13 @@ Palette* MuseScore::newFretboardDiagramPalette()

 void MuseScore::setAdvancedPalette()
      {
-      mscore->getPaletteBox();


I'm not clear on what is going on with this change (or the similar one in setBasicPalette(). Can you describe?

I'm getting the shortcuts from the palette cells, and then setting those shortcuts in the new palette cells. setAdvancedPalette() and setBasicPalette() get called each time the palettes are redrawn, so the shortcuts need to be preserved by saving them before the palette box is cleared, and then setting the shortcuts in the new palette box.

OK, I believe that makes sense. Previously, there was no state that needed preserving - we could just rebuild these from scratch each time we needed to. Now, we need to at least remember the shortcuts assigned. I guess I was thinking somehow we might be reading/writing these on switch if there had been any customization, and the shortcuts would take care of themselves. Would that also have been a valid approach? Any advantages/disadvantages?

I think this would also be valid, but many changes would need to be made to use that approach. There were some problems if the workspace was read each time (palette shortcuts would stop working when the workspace was toggled), so to fix this, the palettes are read only on startup, and after that only the shortcuts are stored. Shortcuts are saved, stored each time the workspace is switched.

OK, thanks for the feedback. I was wondering if the workspace save/restore would be easier, but if not, just keep it the way it is.

MarcSabatella · 2017-08-26T14:41:00Z

mscore/menus.cpp

@@ -1569,8 +1569,10 @@ void MuseScore::setAdvancedPalette()
      preferences.paletteCellList.clear();
      for (int i = 0; i < shortcuts.size(); i++) {
            PaletteCell* cell = cells[i];
-            if (cell->name == "Show More")
+            if (cell->name == "Show More") {


I wonder if it is also worth making this a boolean flag in the cell, set when we create it, rather than keying off the name? Or if perhaps something like checking if cell->element == 0?

MarcSabatella · 2019-10-03T15:18:11Z

There was much to like about this PR - as I recall it did the job as advertised and did so in a fairly clean way. f course, it would need work to update for the new palette implementation, but I actually suspect the amount of work needed would not be huge. The changes to existing files were not the most significant part of this PR, it was the new palette shortcut manager, which would probably still be usable as is.

However, what I also remember is that there was a fair amount of code duplication between the management of shortcuts here and for plugins. The code was modelled after the corresponding plugin code but it was kind of a"TODO" to someday actually refactor it to use a common base class or whatever.

The main differentiator between palette/plugin shortcuts on one hand, and ordinary command shortcuts on the other, is the list of commands is fixed and known at compile time, but plugins and palettes can be added, deleted, etc. We've had any number of bugs come up through mismanagement of plugin shortcuts, it would be good to neither reproduce the already existing bugs (some I guess have been fixed since this PR) nor make it harder to maintain the code in the future.

So I feel this PR is a very good reference for someone wanting to re-implement this, but probably it doesn't make sense to literally start from this branch.

dmitrio95 · 2020-03-11T10:06:40Z

Needs actualization for current master

divya-urs changed the title ~~set palette shortcut~~ Palette Shortcut Jul 30, 2017

divya-urs changed the title ~~Palette Shortcut~~ Palette shortcut Jul 30, 2017

Jojo-Schmitz reviewed Jul 30, 2017

View reviewed changes

lasconic added the work in progress not finished work or not addressed review label Jul 30, 2017

divya-urs force-pushed the palette_shortcut branch from e36f521 to 7e51189 Compare August 13, 2017 16:44

divya-urs force-pushed the palette_shortcut branch from fe7d774 to 837a1e7 Compare August 21, 2017 16:27

MarcSabatella reviewed Aug 25, 2017

View reviewed changes

lasconic removed the work in progress not finished work or not addressed review label Aug 25, 2017

divya-urs force-pushed the palette_shortcut branch from ffb567b to f53c3b3 Compare August 25, 2017 19:02

MarcSabatella reviewed Aug 26, 2017

View reviewed changes

divya-urs force-pushed the palette_shortcut branch from 08f0843 to 0c84403 Compare August 27, 2017 10:29

divya-urs force-pushed the palette_shortcut branch from 0c84403 to eec3df6 Compare November 30, 2017 17:40

palette shortcut

657eef5

divya-urs force-pushed the palette_shortcut branch from eec3df6 to 657eef5 Compare December 2, 2017 07:26

MarcSabatella mentioned this pull request Dec 9, 2017

Navigation of master palette and new score wizard templates #3344

Closed

shoogle mentioned this pull request Jan 28, 2018

Accessibility build for RNIB #3425

Closed

anatoly-os added the RNIB label Sep 4, 2018

MarcSabatella mentioned this pull request Sep 5, 2018

Accessibility of palette search results #3357

Closed

dmitrio95 added the archived PRs that have gone stale but could potentially be revived in the future label Mar 11, 2020

dmitrio95 closed this Mar 11, 2020

MarcSabatella mentioned this pull request Jun 19, 2020

[MU4] Shortcuts #6237

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Palette shortcut #3252

Palette shortcut #3252

divya-urs commented Jul 30, 2017 •

edited

Loading

Jojo-Schmitz Jul 30, 2017

lasconic Jul 30, 2017 •

edited

Loading

Jojo-Schmitz Jul 30, 2017

Jojo-Schmitz commented Aug 22, 2017

divya-urs commented Aug 22, 2017 via email

Jojo-Schmitz commented Aug 23, 2017 •

edited

Loading

divya-urs commented Aug 23, 2017

Jojo-Schmitz commented Aug 23, 2017

divya-urs commented Aug 24, 2017 via email

Jojo-Schmitz commented Aug 24, 2017

MarcSabatella Aug 25, 2017

divya-urs Aug 25, 2017

MarcSabatella Aug 25, 2017

divya-urs Aug 25, 2017

MarcSabatella Aug 25, 2017

MarcSabatella Aug 26, 2017

MarcSabatella commented Oct 3, 2019

dmitrio95 commented Mar 11, 2020

Palette shortcut #3252

Palette shortcut #3252

Conversation

divya-urs commented Jul 30, 2017 • edited Loading

Choose a reason for hiding this comment

lasconic Jul 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jojo-Schmitz commented Aug 22, 2017

divya-urs commented Aug 22, 2017 via email

Jojo-Schmitz commented Aug 23, 2017 • edited Loading

divya-urs commented Aug 23, 2017

Jojo-Schmitz commented Aug 23, 2017

divya-urs commented Aug 24, 2017 via email

Jojo-Schmitz commented Aug 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarcSabatella commented Oct 3, 2019

dmitrio95 commented Mar 11, 2020

divya-urs commented Jul 30, 2017 •

edited

Loading

lasconic Jul 30, 2017 •

edited

Loading

Jojo-Schmitz commented Aug 23, 2017 •

edited

Loading