D28152: KWinRules KCM Redesign

[Nathaniel Graham]

ngraham added a comment.


  This is a huge improvement over the UI of the old one, very nice! However I can't actually get it to work. I target Kate, select "Exact Match" on the window class, and apply some settings, but they don't take effect. Am I doing something wrong? I've restarted KWin after applying this patch.

INLINE COMMENTS

> OptionsComboBox.qml:69
> +
> +    delegate: QQC2.ItemDelegate {
> +        highlighted: optionsCombo.highlightedIndex == index

would be nice if clicking anywhere in the row activated that row's checkbox

> RuleItemDelegate.qml:26
> +
> +Kirigami.AbstractListItem {
> +    id: ruleDelegate

These listitems shouldn't change their background colors on hover, since nothing happens when they're selected. You can see how I did that in https://cgit.kde.org/plasma-desktop.git/commit/?id=5a5e0bc0880b6b21c60b5f874f7eddc2a7939a48

> RulesEditor.qml:43
> +        }
> +        QQC2.ToolButton {
> +            id: showAllButton

Perhaps this should be checked when creating a new rule, or else it looks like there aren't any properties you can apply to the window when creating the rule

> RulesList.qml:54
> +
> +    view: ListView {
> +        id: ruleBookView

Add a centered placeholder message using a disabled level 3 Heading in here when there are no rules

> RulesList.qml:103
> +            ,
> +            Kirigami.Action {
> +                text: i18n("Export")

Disable this when there are no rules

> RulesList.qml:115
> +            Kirigami.Action {
> +                text: i18n("New")
> +                iconName: "list-add-symbolic"

All button text needs to begin with an action verb and end with an ellipsis if more actions are required to complete the action. So this should be "Add New..."

> ValueEditor.qml:136
> +            }
> +            QQC2.Label {
> +                text: i18n("%1 %", Math.round(slider.value))

Give this a minimum width, or else the rowlayout gets wider with every digit added to the number and resizes the combobox next to it

> rulesmodel.cpp:215
> +    if (wmclassWarning()) {
> +        return i18n("You have specified the window class as unimportant.\n"
> +                    "This means the settings will possibly apply to windows from all applications."

Maybe "Exact Match" should be the default settting so this message isn't always shown for every new rule you add

REPOSITORY
  R108 KWin

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

To: iasensio, #plasma, #kwin, #vdg
Cc: ngraham, davidedmundson, hchain, broulik, zzag, kwin, dmenig, manueljlin, Orage, cacarry, LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, Ghost6, jraleigh, zachus, fbampaloukas, squeakypancakes, alexde, IohannesPetros, GB_2, mkulinski, trickyricky26, ragreen, jackyalcine, iodelay, crozbo, ndavis, bwowk, ZrenBot, firef, alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, hardening, romangg, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwin/attachments/20200415/4370c9e2/attachment-0001.html>