Commit c24340bd authored by David Redondo's avatar David Redondo 🏎

[kcms/keys] Handle conflicts in the kcm

Previously conflicts where handled in KeySequenceItem where inputs were being
compared against the current active shortcut configuration. For simple
configuration actions this is good enough but inconvenient for more complicated
user interaction. If action A has key sequence K, the user removes K from A and
assigns it to another action B, they would prviously be prompted that K is
assigned to A, even though after clicking apply there were no changes.

Doing conflict detection ourselves we can keep the model consistent and prompt
for conflicts against the current state of the models of the kcm. Also solves
a bug where it would fail to activate a default key sequence when it was
assigned to another action because the checkbox would not check for conflicts.
parent 81da82a6
......@@ -27,22 +27,6 @@ BaseModel::BaseModel(QObject *parent)
{
}
void BaseModel::toggleDefaultShortcut(const QModelIndex &index, const QKeySequence &shortcut, bool enabled)
{
if (!checkIndex(index, QAbstractItemModel::CheckIndexOption::IndexIsValid) || !index.parent().isValid()) {
return;
}
qCDebug(KCMKEYS) << "Default shortcut" << index << shortcut << enabled;
Action &a = m_components[index.parent().row()].actions[index.row()];
if (enabled) {
a.activeShortcuts.insert(shortcut);
} else {
a.activeShortcuts.remove(shortcut);
}
Q_EMIT dataChanged(index, index, {ActiveShortcutsRole, DefaultShortcutsRole});
}
void BaseModel::addShortcut(const QModelIndex &index, const QKeySequence &shortcut)
{
if (!checkIndex(index, QAbstractItemModel::CheckIndexOption::IndexIsValid) || !index.parent().isValid()) {
......
......@@ -65,7 +65,6 @@ public:
BaseModel(QObject *parent = nullptr);
Q_INVOKABLE void toggleDefaultShortcut(const QModelIndex &index, const QKeySequence &shortcut, bool enabled);
Q_INVOKABLE void addShortcut(const QModelIndex &index, const QKeySequence &shortcut);
Q_INVOKABLE void disableShortcut(const QModelIndex &index, const QKeySequence &shortcut);
Q_INVOKABLE void changeShortcut(const QModelIndex &index, const QKeySequence &oldShortcut, const QKeySequence &newShortcut);
......
......@@ -43,8 +43,6 @@
#include "shortcutsmodel.h"
#include "standardshortcutsmodel.h"
K_PLUGIN_CLASS_WITH_JSON(KCMKeys, "kcm_keys.json")
KCMKeys::KCMKeys(QObject *parent, const QVariantList &args)
......@@ -199,4 +197,72 @@ QString KCMKeys::urlFilename(const QUrl &url)
return url.fileName();
}
QModelIndex KCMKeys::conflictingIndex(const QKeySequence &keySequence)
{
for (int i = 0; i < m_shortcutsModel->rowCount(); ++i) {
const QModelIndex componentIndex = m_shortcutsModel->index(i, 0);
for (int j = 0; j < m_shortcutsModel->rowCount(componentIndex); ++j) {
const QModelIndex actionIndex = m_shortcutsModel->index(j, 0, componentIndex);
if (m_shortcutsModel->data(actionIndex, BaseModel::ActiveShortcutsRole).value<QSet<QKeySequence>>().contains(keySequence)) {
return m_shortcutsModel->mapToSource(actionIndex);
}
}
}
return QModelIndex();
}
void KCMKeys::requestKeySequence(QQuickItem *context, const QModelIndex &index,
const QKeySequence &newSequence, const QKeySequence &oldSequence)
{
qCDebug(KCMKEYS) << index << "wants" << newSequence << "instead of" << oldSequence;
const QModelIndex conflict = conflictingIndex(newSequence);
if (!conflict.isValid()) {
auto model = const_cast<BaseModel*>(static_cast<const BaseModel*>(index.model()));
if (!oldSequence.isEmpty()) {
model->changeShortcut(index, oldSequence, newSequence);
} else {
model->addShortcut(index, newSequence);
}
return;
}
qCDebug(KCMKEYS) << "Found conflict for" << newSequence << conflict;
const bool isStandardAction = conflict.parent().data(BaseModel::SectionRole).toString() == i18n("Common Actions");
const QString actionName = conflict.data().toString();
const QString componentName = conflict.parent().data().toString();
const QString keysString = newSequence.toString(QKeySequence::NativeText);
const QString message = isStandardAction ? i18nc("%2 is the name of a category inside the 'Common Actions' section",
"Shortcut %1 is already assigned to the common %2 action '%3'.\nDo you want to reassign it?", keysString, componentName, actionName)
: i18n("Shortcut %1 is already assigned to action '%2' of %3.\nDo you want to reassign it?", keysString, actionName, componentName);
const QString title = i18nc("@title:window", "Found conflict");
auto dialog = new QDialog;
if (context && context->window()) {
dialog->winId(); // so it creates windowHandle
dialog->windowHandle()->setTransientParent(QQuickRenderControl::renderWindowFor(context->window()));
}
dialog->setWindowModality(Qt::WindowModal);
dialog->setAttribute(Qt::WA_DeleteOnClose);
KMessageBox::createKMessageBox(dialog, new QDialogButtonBox(QDialogButtonBox::Yes | QDialogButtonBox::No, dialog),
QMessageBox::Question, message, {}, QString(), nullptr, KMessageBox::NoExec);
dialog->show();
connect(dialog, &QDialog::finished, this, [this, index, conflict, newSequence, oldSequence] (int result) {
auto model = const_cast<BaseModel*>(static_cast<const BaseModel*>(index.model()));
if (result != QDialogButtonBox::Yes) {
// Also emit if we are not changing anything, to force the frontend to update and be consistent
// with the model. It is currently out of sync because it reflects the user input that
// was rejected now.
Q_EMIT model->dataChanged(index, index, {BaseModel::ActiveShortcutsRole, BaseModel::CustomShortcutsRole});
return;
}
const_cast<BaseModel*>(static_cast<const BaseModel*>(conflict.model()))->disableShortcut(conflict, newSequence);
if (!oldSequence.isEmpty()) {
model->changeShortcut(index, oldSequence, newSequence);
} else {
model->addShortcut(index, newSequence);
}
});
}
#include "kcm_keys.moc"
......@@ -21,6 +21,7 @@
#ifndef KCM_KEYS_H
#define KCM_KEYS_H
#include <QKeySequence>
#include <QObject>
#include <KQuickAddons/ConfigModule>
......@@ -46,6 +47,9 @@ public:
void load() override;
void save() override;
Q_INVOKABLE void requestKeySequence(QQuickItem *context, const QModelIndex &index,
const QKeySequence &newSequence, const QKeySequence &oldSequence = QKeySequence());
Q_INVOKABLE void writeScheme(const QUrl &url);
Q_INVOKABLE void loadScheme(const QUrl &url);
Q_INVOKABLE QVariantList defaultSchemes() const;
......@@ -64,6 +68,7 @@ Q_SIGNALS:
private:
void setError(const QString &errorMessage);
QModelIndex conflictingIndex(const QKeySequence &keySequence);
QString m_lastError;
FilteredShortcutsModel *m_filteredModel;
......
......@@ -97,7 +97,13 @@ Kirigami.AbstractListItem {
QQC2.CheckBox {
checked: activeShortcuts.indexOf(modelData) != -1
text: modelData
onToggled: originalIndex.model.toggleDefaultShortcut(originalIndex, modelData, checked)
onToggled: {
if (checked) {
kcm.requestKeySequence(this, originalIndex, modelData)
} else {
originalIndex.model.disableShortcut(originalIndex, modelData)
}
}
}
}
}
......@@ -117,8 +123,9 @@ Kirigami.AbstractListItem {
KeySequenceItem {
keySequence: modelData
showClearButton: false
checkForConflictsAgainst: ShortcutType.None
onCaptureFinished: {
originalIndex.model.changeShortcut(originalIndex, modelData, keySequence)
kcm.requestKeySequence(this, originalIndex, keySequence, modelData)
}
}
QQC2.Button {
......@@ -154,8 +161,9 @@ Kirigami.AbstractListItem {
signal finished
KeySequenceItem {
showClearButton: false
checkForConflictsAgainst: ShortcutType.None
onCaptureFinished: {
originalIndex.model.addShortcut(originalIndex, keySequence)
kcm.requestKeySequence(this, originalIndex, keySequence)
parent.finished()
}
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment