D28152: KWinRules KCM Redesign

ngraham added a comment.


  Wow, this is waaaaaay better. The workflow is really smooth. It's, like, incredibly great now. Also, the rules actually work for me now! So that's nice.
  
  I really only have nitpicks:

INLINE COMMENTS

> RuleItemDelegate.qml:63
> +            id: itemEnabled
> +            icon.name: "list-remove"
> +            opacity: (ruleDelegate.hovered && model.selectable) ? 1 : 0

let's use `edit-delete` for this, as that uses a trashcan icon and we're moving towards using that everywhere

> RulesEditor.qml:42
> +        model: enabledRulesModel
> +        delegate: RuleItemDelegate {}
> +        section {

the list items are back to changing their background on hover, which isn't necessary

> RulesEditor.qml:48
> +
> +        Kirigami.Heading {
> +            level: 3

Might be a good idea to move the "Add properties... button right below this text so it's obvious to the user now they add more properties. When the placeholder message is not shown, the button could just be there at the bottom of the list, and maybe could change its text to "Add more properties..."

The reason for this is to make it easy for the user to understand what they need to do. We do this in the online accounts KCM: F8245459: Screenshot_20200418_112625.png <https://phabricator.kde.org/F8245459>

> RulesEditor.qml:73
> +
> +    footer: RowLayout {
> +        QQC2.Button {

I would make this footer a header to mimic the natural flow. Typically you'll click the "detectwindow properties" button first to target a window, which means it should be at the top of the window to suggest that it's the first thing you want to click on.

> RulesEditor.qml:75
> +        QQC2.Button {
> +            text: i18n("Detect window properties")
> +            icon.name: "edit-find"

Button titles use Title Case ("Detect Window Properties")

> RulesEditor.qml:96
> +        QQC2.Button {
> +            text: i18n("Add properties")
> +            icon.name: "list-add-symbolic"

use Title Case and add ellipses ("Add Properties...")

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