Merge pull request #26775 from mathesoncalum/450-porting-2

Porting more PRs to 4.5.0

src/app/configs/data/shortcuts.xml

@@ -898,6 +898,10 @@
     <key>toggle-piano-keyboard</key>
     <seq>P</seq>
     </SC>
+  <SC>
+    <key>toggle-percussion-panel</key>
+    <seq>O</seq>
+    </SC>
   <SC>
     <key>next-score</key>
     <std>20</std>

src/app/configs/data/shortcuts_azerty.xml

@@ -924,6 +924,10 @@
     <key>toggle-piano-keyboard</key>
     <seq>P</seq>
     </SC>
+  <SC>
+    <key>toggle-percussion-panel</key>
+    <seq>O</seq>
+    </SC>
   <SC>
     <key>next-score</key>
     <std>20</std>

src/app/configs/data/shortcuts_mac.xml

@@ -899,6 +899,10 @@
     <key>toggle-piano-keyboard</key>
     <seq>P</seq>
     </SC>
+  <SC>
+    <key>toggle-percussion-panel</key>
+    <seq>O</seq>
+    </SC>
   <SC>
     <key>next-score</key>
     <std>20</std>

src/appshell/internal/applicationuiactions.cpp

@@ -39,6 +39,7 @@ using namespace muse::dock;
 static const ActionCode FULL_SCREEN_CODE("fullscreen");
 static const ActionCode TOGGLE_NAVIGATOR_ACTION_CODE("toggle-navigator");
 static const ActionCode TOGGLE_BRAILLE_ACTION_CODE("toggle-braille-panel");
+static const ActionCode TOGGLE_PERCUSSION_PANEL_ACTION_CODE("toggle-percussion-panel");
 
 const UiActionList ApplicationUiActions::m_actions = {
     UiAction("quit",
@@ -194,14 +195,13 @@ const UiActionList ApplicationUiActions::m_actions = {
              TranslatableString("action", "Show/hide piano keyboard"),
              Checkable::Yes
              ),
-    // still in development
-    // UiAction("toggle-percussion-panel",
-    //          mu::context::UiCtxNotationOpened,
-    //          mu::context::CTX_ANY,
-    //          TranslatableString("action", "Percussion"),
-    //          TranslatableString("action", "Show/hide percussion panel"),
-    //          Checkable::Yes
-    //          ),
+    UiAction(TOGGLE_PERCUSSION_PANEL_ACTION_CODE,
+             mu::context::UiCtxProjectOpened,
+             mu::context::CTX_NOTATION_OPENED,
+             TranslatableString("action", "Percussion"),
+             TranslatableString("action", "Show/hide percussion panel"),
+             Checkable::Yes
+             ),
     UiAction("toggle-scorecmp-tool",
              mu::context::UiCtxProjectOpened,
              mu::context::CTX_NOTATION_OPENED,
@@ -248,6 +248,10 @@ void ApplicationUiActions::init()
     dockWindowProvider()->windowChanged().onNotify(this, [this]() {
         listenOpenedDocksChanged(dockWindowProvider()->window());
     });
+
+    notationConfiguration()->useNewPercussionPanelChanged().onNotify(this, [this]() {
+        m_actionEnabledChanged.send({ TOGGLE_PERCUSSION_PANEL_ACTION_CODE });
+    });
 }
 
 void ApplicationUiActions::listenOpenedDocksChanged(IDockWindow* window)
@@ -280,11 +284,11 @@ const muse::ui::UiActionList& ApplicationUiActions::actionsList() const
 
 bool ApplicationUiActions::actionEnabled(const UiAction& act) const
 {
-    if (!m_controller->canReceiveAction(act.code)) {
-        return false;
+    if (act.code == TOGGLE_PERCUSSION_PANEL_ACTION_CODE) {
+        return notationConfiguration()->useNewPercussionPanel();
     }
 
-    return true;
+    return m_controller->canReceiveAction(act.code);
 }
 
 bool ApplicationUiActions::actionChecked(const UiAction& act) const
@@ -340,7 +344,7 @@ const QMap<ActionCode, DockName>& ApplicationUiActions::toggleDockActions()
         { "toggle-timeline", TIMELINE_PANEL_NAME },
         { "toggle-mixer", MIXER_PANEL_NAME },
         { "toggle-piano-keyboard", PIANO_KEYBOARD_PANEL_NAME },
-        { "toggle-percussion-panel", PERCUSSION_PANEL_NAME },
+        { TOGGLE_PERCUSSION_PANEL_ACTION_CODE, PERCUSSION_PANEL_NAME },
 
         { "toggle-statusbar", NOTATION_STATUSBAR_NAME },
     };

src/appshell/internal/applicationuiactions.h

@@ -29,6 +29,7 @@
 #include "async/asyncable.h"
 #include "ui/imainwindow.h"
 #include "view/preferences/braillepreferencesmodel.h"
+#include "notation/inotationconfiguration.h"
 
 #include "dockwindow/idockwindowprovider.h"
 
@@ -39,6 +40,7 @@ class ApplicationUiActions : public muse::ui::IUiActionsModule, public muse::Inj
     muse::Inject<muse::dock::IDockWindowProvider> dockWindowProvider = { this };
     muse::Inject<IAppShellConfiguration> configuration = { this };
     muse::Inject<braille::IBrailleConfiguration> brailleConfiguration = { this };
+    muse::Inject<mu::notation::INotationConfiguration> notationConfiguration = { this };
 
 public:
     ApplicationUiActions(std::shared_ptr<ApplicationActionController> controller, const muse::modularity::ContextPtr& iocCtx);

src/appshell/view/appmenumodel.cpp

@@ -260,7 +260,7 @@ MenuItem* AppMenuModel::makeViewMenu()
         makeMenuItem("toggle-timeline"),
         makeMenuItem("toggle-mixer"),
         makeMenuItem("toggle-piano-keyboard"),
-        // makeMenuItem("toggle-percussion-panel"), // still in development
+        makeMenuItem("toggle-percussion-panel"),
         makeMenuItem("playback-setup"),
         //makeMenuItem("toggle-scorecmp-tool"), // not implemented
         makeSeparator(),

src/engraving/dom/instrchange.cpp

@@ -86,70 +86,72 @@ void InstrumentChange::setInstrument(const Instrument& i)
 
 void InstrumentChange::setupInstrument(const Instrument* instrument)
 {
-    if (m_init) {
-        Fraction tickStart = segment()->tick();
-        Part* part = staff()->part();
-        Interval oldV = part->instrument(tickStart)->transpose();
-        Interval oldKv = staff()->transpose(tickStart);
-        Interval v = instrument->transpose();
-        bool concPitch = style().styleB(Sid::concertPitch);
-
-        // change the clef for each staff
-        for (size_t i = 0; i < part->nstaves(); i++) {
-            ClefType oldClefType = concPitch ? part->instrument(tickStart)->clefType(i).concertClef
-                                   : part->instrument(tickStart)->clefType(i).transposingClef;
-            ClefType newClefType = concPitch ? instrument->clefType(i).concertClef
-                                   : instrument->clefType(i).transposingClef;
-            // Introduce cleff change only if the new clef *symbol* is different from the old one
-            if (ClefInfo::symId(oldClefType) != ClefInfo::symId(newClefType)) {
-                // If instrument change is at the start of a measure, use the measure as the element, as this will place the instrument change before the barline.
-                EngravingItem* element = rtick().isZero() ? toEngravingItem(findMeasure()) : toEngravingItem(this);
-                score()->undoChangeClef(part->staff(i), element, newClefType, true);
-            }
+    if (!m_init) {
+        return;
+    }
+
+    Fraction tickStart = segment()->tick();
+    Part* part = staff()->part();
+    Interval oldV = part->instrument(tickStart)->transpose();
+    Interval oldKv = staff()->transpose(tickStart);
+    Interval v = instrument->transpose();
+    bool concPitch = style().styleB(Sid::concertPitch);
+
+    // change the clef for each staff
+    for (size_t i = 0; i < part->nstaves(); i++) {
+        ClefType oldClefType = concPitch ? part->instrument(tickStart)->clefType(i).concertClef
+                               : part->instrument(tickStart)->clefType(i).transposingClef;
+        ClefType newClefType = concPitch ? instrument->clefType(i).concertClef
+                               : instrument->clefType(i).transposingClef;
+        // Introduce cleff change only if the new clef *symbol* is different from the old one
+        if (ClefInfo::symId(oldClefType) != ClefInfo::symId(newClefType)) {
+            // If instrument change is at the start of a measure, use the measure as the element, as this will place the instrument change before the barline.
+            EngravingItem* element = rtick().isZero() ? toEngravingItem(findMeasure()) : toEngravingItem(this);
+            score()->undoChangeClef(part->staff(i), element, newClefType, true);
         }
+    }
 
-        // Change key signature if necessary. CAUTION: not necessary in case of octave-transposing!
-        if ((v.chromatic - oldV.chromatic) % 12) {
-            for (size_t i = 0; i < part->nstaves(); i++) {
-                if (!part->staff(i)->keySigEvent(tickStart).isAtonal()) {
-                    KeySigEvent ks;
-                    // Check, if some key signature is already there, if no, mark new one "for instrument change"
-                    Segment* seg = segment()->prev1(SegmentType::KeySig);
-                    voice_idx_t voice = part->staff(i)->idx() * VOICES;
-                    KeySig* ksig = seg ? toKeySig(seg->element(voice)) : nullptr;
-                    bool forInstChange = !(ksig && ksig->tick() == tickStart && !ksig->generated());
-                    ks.setForInstrumentChange(forInstChange);
-                    Key cKey = part->staff(i)->concertKey(tickStart);
-                    ks.setConcertKey(cKey);
-                    score()->undoChangeKeySig(part->staff(i), tickStart, ks);
-                }
+    // Change key signature if necessary. CAUTION: not necessary in case of octave-transposing!
+    if ((v.chromatic - oldV.chromatic) % 12) {
+        for (size_t i = 0; i < part->nstaves(); i++) {
+            if (!part->staff(i)->keySigEvent(tickStart).isAtonal()) {
+                KeySigEvent ks;
+                // Check, if some key signature is already there, if no, mark new one "for instrument change"
+                Segment* seg = segment()->prev1(SegmentType::KeySig);
+                voice_idx_t voice = part->staff(i)->idx() * VOICES;
+                KeySig* ksig = seg ? toKeySig(seg->element(voice)) : nullptr;
+                bool forInstChange = !(ksig && ksig->tick() == tickStart && !ksig->generated());
+                ks.setForInstrumentChange(forInstChange);
+                Key cKey = part->staff(i)->concertKey(tickStart);
+                ks.setConcertKey(cKey);
+                score()->undoChangeKeySig(part->staff(i), tickStart, ks);
             }
         }
+    }
 
-        // change instrument in all linked scores
-        for (EngravingObject* se : linkList()) {
-            InstrumentChange* lic = static_cast<InstrumentChange*>(se);
-            Instrument* newInstrument = new Instrument(*instrument);
-            lic->score()->undo(new ChangeInstrument(lic, newInstrument));
-        }
+    // change instrument in all linked scores
+    for (EngravingObject* se : linkList()) {
+        InstrumentChange* lic = static_cast<InstrumentChange*>(se);
+        Instrument* newInstrument = new Instrument(*instrument);
+        lic->score()->undo(new ChangeInstrument(lic, newInstrument));
+    }
 
-        // transpose for current score only
-        // this automatically propagates to linked scores
-        if (part->instrument(tickStart)->transpose() != oldV) {
-            auto i = part->instruments().upper_bound(tickStart.ticks());          // find(), ++i
-            Fraction tickEnd;
-            if (i == part->instruments().end()) {
-                tickEnd = Fraction(-1, 1);
-            } else {
-                tickEnd = Fraction::fromTicks(i->first);
-            }
-            score()->transpositionChanged(part, oldKv, tickStart, tickEnd);
+    // transpose for current score only
+    // this automatically propagates to linked scores
+    if (part->instrument(tickStart)->transpose() != oldV) {
+        auto i = part->instruments().upper_bound(tickStart.ticks());              // find(), ++i
+        Fraction tickEnd;
+        if (i == part->instruments().end()) {
+            tickEnd = Fraction(-1, 1);
+        } else {
+            tickEnd = Fraction::fromTicks(i->first);
         }
-
-        //: The text of an "instrument change" marking. It is an instruction to the player to switch to another instrument.
-        const String newInstrChangeText = muse::mtrc("engraving", "To %1").arg(instrument->trackName());
-        undoChangeProperty(Pid::TEXT, TextBase::plainToXmlText(newInstrChangeText));
+        score()->transpositionChanged(part, oldKv, tickStart, tickEnd);
     }
+
+    //: The text of an "instrument change" marking. It is an instruction to the player to switch to another instrument.
+    const String newInstrChangeText = muse::mtrc("engraving", "To %1").arg(instrument->trackName());
+    undoChangeProperty(Pid::TEXT, TextBase::plainToXmlText(newInstrChangeText));
 }
 
 //---------------------------------------------------------

src/framework/accessibility/iaccessible.h

@@ -48,6 +48,7 @@ class IAccessible
         Panel,
         StaticText,
         EditableText,
+        SilentRole, // avoids reading "button", "text", etc. after item name
         Button,
         CheckBox,
         RadioButton,

src/framework/accessibility/internal/accessibleiteminterface.cpp

@@ -156,6 +156,10 @@ QAccessible::State AccessibleItemInterface::state() const
         state.focusable = true;
         state.focused = item->accessibleState(IAccessible::State::Focused);
     } break;
+    case IAccessible::Role::SilentRole: {
+        state.focusable = true;
+        state.focused = item->accessibleState(IAccessible::State::Focused);
+    } break;
     case IAccessible::Role::List: {
         state.active = item->accessibleState(IAccessible::State::Active);
     } break;
@@ -217,6 +221,23 @@ QAccessible::Role AccessibleItemInterface::role() const
     case IAccessible::Role::Dialog: return QAccessible::Dialog;
     case IAccessible::Role::Panel: return QAccessible::Pane;
     case IAccessible::Role::StaticText: return QAccessible::StaticText;
+    case IAccessible::Role::SilentRole: {
+        // See https://doc.qt.io/qt-5/qaccessible.html#Role-enum
+        // We want the screen reader to say the name of the current item and
+        // nothing else (i.e. not the name followed by "button" or "text").
+#if defined(Q_OS_MACOS)
+        // Good on macOS with VoiceOver.
+        return QAccessible::StaticText;
+        // VoiceOver gives unwanted additional output if ListItem is used, and it
+        // doesn't work at all if the role is TreeItem or Cell.
+#else
+        // Good on Windows with Narrator, NVDA, or JAWS; and on Linux with Orca.
+        return QAccessible::ListItem;
+        // Orca is equally happy with the roles TreeItem or Cell, but these cause
+        // unwanted additional ouput on Windows. StaticText causes unwanted
+        // additional output on both Linux and Windows.
+#endif
+    }
     case IAccessible::Role::EditableText: return QAccessible::EditableText;
     case IAccessible::Role::Button: return QAccessible::Button;
     case IAccessible::Role::CheckBox: return QAccessible::CheckBox;

src/framework/shortcuts/qml/Muse/Shortcuts/EditShortcutDialogContent.qml

@@ -44,6 +44,7 @@ Item {
 
     signal saveRequested()
     signal cancelRequested()
+    signal clearRequested()
     signal keyPressed(var event)
 
     anchors.fill: parent
@@ -138,6 +139,17 @@ Item {
             navigationPanel.section: root.navigationSection
             navigationPanel.order: 2
 
+            FlatButton {
+                text: qsTrc("global", "Clear")
+                buttonRole: ButtonBoxModel.CustomRole
+                buttonId: ButtonBoxModel.Clear
+                isLeftSide: true
+
+                onClicked: {
+                    root.clearRequested()
+                }
+            }
+
             onStandardButtonClicked: function(buttonId) {
                 if (buttonId === ButtonBoxModel.Cancel) {
                     root.cancelRequested()

src/framework/shortcuts/qml/Muse/Shortcuts/StandardEditShortcutDialog.qml

@@ -63,7 +63,8 @@ StyledDialogView {
 
         headerText: qsTrc("shortcuts", "Define keyboard shortcut")
 
-        originShortcutText: editShortcutModel.originSequence
+        //! NOTE: There's no need to actually clear the origin shortcut, we can simply hide it for aesthetic purposes...
+        originShortcutText: !editShortcutModel.cleared ? editShortcutModel.originSequence : ""
         newShortcutText: editShortcutModel.newSequence
         informationText: editShortcutModel.conflictWarning
 
@@ -76,6 +77,10 @@ StyledDialogView {
             root.reject()
         }
 
+        onClearRequested: {
+            editShortcutModel.clear()
+        }
+
         onKeyPressed: function(event) {
             editShortcutModel.inputKey(event.key, event.modifiers)
         }

src/framework/shortcuts/view/editshortcutmodel.cpp

@@ -63,7 +63,10 @@ void EditShortcutModel::load(const QVariant& originShortcut, const QVariantList&
     m_originSequence = originShortcutMap.value("sequence").toString();
     m_originShortcutTitle = originShortcutMap.value("title").toString();
 
+    m_cleared = false;
+
     emit originSequenceChanged();
+    emit clearedChanged();
 }
 
 void EditShortcutModel::clearNewSequence()
@@ -110,6 +113,14 @@ void EditShortcutModel::inputKey(Qt::Key key, Qt::KeyboardModifiers modifiers)
     emit newSequenceChanged();
 }
 
+void EditShortcutModel::clear()
+{
+    clearNewSequence();
+    m_cleared = true;
+    emit originSequenceChanged();
+    emit clearedChanged();
+}
+
 bool EditShortcutModel::isShiftAllowed(Qt::Key key)
 {
     if (key >= Qt::Key_A && key <= Qt::Key_Z) {
@@ -215,8 +226,8 @@ QString EditShortcutModel::conflictWarning() const
 void EditShortcutModel::trySave()
 {
     QString newSequence = this->newSequence();
-
-    if (m_originSequence == newSequence) {
+    const bool alreadyEmpty = originSequenceInNativeFormat().isEmpty() && m_cleared;
+    if (alreadyEmpty || m_originSequence == newSequence) {
         return;
     }