D28744: Rewrite of the global shortcuts kcm

[David Redondo]

davidre added inline comments.

INLINE COMMENTS

> filteredmodel.cpp:41
> +    bool displayMatches = index.data(Qt::DisplayRole).toString().contains(m_filter, Qt::CaseInsensitive);
> +    if (!source_parent.isValid() || displayMatches) {
> +        return displayMatches;

If it's a toplevel item we can just return if the name matches. Recursive filtering will take care of the rest

> filteredmodel.cpp:45
> +
> +    if (index.parent().data(Qt::DisplayRole).toString().contains(m_filter, Qt::CaseInsensitive)) {
> +        return true;

No if the parent matches but the child not, the child is filtered. Recursive filtering makes the parent show if it doesn't match but the child does

> ShortcutActionDelegate.qml:58
> +                        if (model.activeShortcuts.length != 0) {
> +                            return model.display + ": " + model.activeShortcuts.map(s => kcm.keySequenceToString(s)).join(", ")
> +                        } else {

Shouldn't QKeySequence::NativeText take care of that or because of the list format?

> main.qml:53
> +                enabled: exportWarning.visible
> +                function onNeedsSaveChanged () {
> +                    exportWarning.visible = kcm.needsSave

The way I used it caused a binding loop

  Binding on visible {
                  when: exportWarning.visible
                  value: kcm.needsSave
                  restoreMode: Binding.RestoreValue
              }

> main.qml:141
> +                    icon.name: "list-add"
> +                    text: i18n("Add application...")
> +                    onClicked: {

I know...
I will look into what's the most reliable way of deleting things...

> metadata.desktop:4
> +
> +Comment=Global Keyboard Shortcuts
> +

It's the same as in the main desktop file. What will be the user visible one?

> shortcutsmodel.cpp:298
> +{
> +   if (!checkIndex(index) || !index.parent().isValid())  {
> +        return;

I think that's wrong way round here

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D28744

To: davidre, #vdg, #plasma
Cc: broulik, davidedmundson, nicolasfella, ngraham, iasensio, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200414/66e4c307/attachment-0001.html>