Super awesome!

I don't know if it would be best at this stage to create a RFC code review or upload it to a new <user> branch.

Either works

Let me know if you want me to look at any of these specific tasks.

This looks much better! Only showing preferences that are part of the rule by default for already existing rules is a good idea too imo.

The biggest UI issue I see with window rules is not introduced by this change but maybe this can now easily be solved:
I think the biggest problem users will have with figuring out how to use window rules to change a specific behaviour of a window is understanding the window matching part. I think for non-technical folks it is unintuitive that the question "What does this rule apply to?" is part of the rule itself. This problem is made more grave by the string "window matching" that doesn't necessarily convey that this decides what the rule is applied to. Overall the window matching section is quite hard to understand because terminologies like "window class", "window types" and "window role" aren't easily understood.

Here is a mockup that contains some ideas to solve this:

• Separated "window matching" from the rest.
• Changed "window class" to an application selection
• Only display conditions that are in use: I am a bit unhappy that I solved this differently than with the "Show all rules"-button. I went with the approach of adding/removing conditions because I thought having a checkbox to enable/disable conditions wasn't easy enough to understand. I especially thought this would be a problem when a new rule is created and the user is faced with five unchecked conditions.

I really like your mock-up! Some of the ideas also crossed my mind when I first approached the KCM but I couldn't visualize them in a nice way like you did. I'm adding you're ideas to the task to-do list, although maybe they'll have to wait (with some others) until we get a first functional patch, and be added in the "polish" phase.

• Separated "window matching" from the rest.

Technically, I think is not too hard to achieve using the same backend model, but just having two views with different proxy filters, one for "Window Matching" properties and another one for the effects.

• Changed "window class" to an application selection

I have more doubts about this one, since I don't know easy or feasible is to map applications to its wmclass string, but anyway a must addition.

• Only display conditions that are in use: I am a bit unhappy that I solved this differently than with the "Show all rules"-button. I went with the approach of adding/removing conditions because I thought having a checkbox to enable/disable conditions wasn't easy enough to understand. I especially thought this would be a problem when a new rule is created and the user is faced with five unchecked conditions.

I find your approach (Add/Remove) better from an user point of view, although the problem I see is how to present the possible effects that can be added to the user, that is, what happens when you press "+ Add.... Have you something in mind?

In T12729#221631, @davidedmundson wrote:

Let me know if you want me to look at any of these specific tasks.

Thanks! I'm currently struggling a bit with the KConfig stuff and how to undo the changes to a previous state (before Apply).
I think I don't understand how it really works, and maybe my approach here is not the best one. That would be my first milestone for considering it at minimal working state and submit a RFC patch.

I'm adding you're ideas to the task to-do list

I am happy to hear that!

I don't know easy or feasible is to map applications to its wmclass string

I was wondering about that. Hopefully there is an easy way. But ye, this is one of the things that can be done any time later I think.

• Only display conditions that are in use: I am a bit unhappy that I solved this differently than with the "Show all rules"-button. I went with the approach of adding/removing conditions because I thought having a checkbox to enable/disable conditions wasn't easy enough to understand. I especially thought this would be a problem when a new rule is created and the user is faced with five unchecked conditions.

I find your approach (Add/Remove) better from an user point of view, although the problem I see is how to present the possible effects that can be added to the user, that is, what happens when you press "+ Add.... Have you something in mind?

I was thinking about this some more but didn't come up with a way to solve this better than in my mockup. I think it might be fine though. For the "conditions" list the "Add conditions" button would have a dropdown with entries like "Add application condition", "Add window title condition", "Add window type condition", … This way no secondary window is necessary to create a rule. I would guess that most of the time only one or two conditions would be added this way and because there are only a few possible conditions to choose from a dropdown is a great fit here.
For the "rule effects" list the current behaviour has the same benefit of not making a secondary window necessary. The big list of possible "effects" needs to be somewhere and putting it in another window that spawns when clicking an "Add …" button wouldn't make the UI more understandable IMO (although this wouldn't be a bad way to solve this either). The workflow of activating effects in a rule by clicking its checkbox also makes sense IMO (contrary to the "conditions" list).
So yea, although it is a bit unfortunate to have two different ways to add objects to a list in the same window it might still be the best solution for the window rule modification/creation.

Hi @iasensio, great work on the UI redesign! I currently have a patch https://phabricator.kde.org/D27682 to port the existing kwinrules KCM to KConfig XT. Hopefully this can help bridge existing configuration with your model, as well as import/export.

Hi there! Thanks, I've been watching your patch and currently following it to see how it evolves.

The current models here extract the info directly by directly reading the KConfig groups and properties. I think that having a specific settings object like RuleBookSettings can simplify this, and make it more coherent, but I don't know much about KConfigXT.
I've seen that it generates a pair of methods for each config property (prop()/setProp()) Does it also provide a similar API to KConfig for the properties like readEntry(prop) / writeEntry(prop)? If that is the case, then I think the integration with your patch can be quite straightforward.

I also wonder if the XML schema for KConfigXT can be somehow extended to include extra custom properties. Then, the model could import the rules' properties directly from the schema making it fully flexible. But that is a discussion for the future.

I'm going to try closing the pending features one at a time and then see which is the best path forward.

KConfig XT is based on KConfig, and there is an underlying KConfig Object with entries/groups. But the entire point of KConfig XT is not to use them, and use the auto generated accessors instead. The kconfig compiler can also make RuleBookSettings a QObject with Q_PROPERTIES and signals that you can connect to your model.

Hi @iasensio , I can help with porting your code to KConfig XT . Do you mind if I rebase your branch and make some modifications ?

Hi there, sorry, I just read your comment.

Thanks for your help! I have already rebased it locally and started the porting.
Currently, the rule edition and the new and remove methods are working.
I'm now just re-adding the functionallity bit by bit, following by move, import and export.

Editing to add that there are also some small refactoring bits that I wanted to do before, and seem to be easier now.

These evening, after that, I'll upload the new branch, and I will reach you for some help on the following :)

Great!
I was thinking you might want to use pointer to RuleSettings members instead of config item name strings inside of RuleItem. Also maybe it would be worthwhile to make RuleItem a template (I think it doesn't need to be a QObject) instead of your Type enum.

BTW, are you aware of https://phabricator.kde.org/D24821 ? I could change the kcfgc file to make RuleBookSettings a QObject with signals that a ManagedConfigModule could use to handle the load/save/defaults buttons automatically

I must admit that those suggestions kind of overwhelm me and my skills. 😅

I'm thinking of submitting a RFC patch (on the current working version (still polishing some bits), not to be merged right away, but so more eyes can try it easily, both UI and code-wise. The rest of the integration steps and improvements you suggest, and some others can be added later, on the same feature branch.

Otherwise I fear that I'll start losing any sense of progress. I hope that you understand and it sounds sane to you.

Since they tend to get lost on code review, I'm pasting some of @broulik 's pending comments and my answers here. They are about some UI decisions I would like to check with the rest of the VDG:

• I think the rule name should be editable in the list rather than being a "Description" field on the window matching section. Took me a while to figure out how to rename a rule

It's implemented now, but I find it a bit quirky due to how TextField behaves, and I might need some hints on that.

• Generally we use regular buttons rather than tool buttons below the view for "New"

Here I feel that the non-flat buttons can give a more "busy" feeling when close to the KCM buttons, while flat ones make it more clean, but I'm not strong against it:

• Not sure about the use of icons for the window types, perhaps a ComboBox with multi selection is better?

I've tried it, but I see a problem about how to show the selected items when the combo is closed
Current:

• We generally don't use switches in UI

I know. In this case I think a checkbox is out of the question visually and semantically, so an alternative can be the yes/no radio buttons like in the old KCM. I still slightly prefer the switch, but the radios doesn't look that bad either.

I've tried it, but I see a problem about how to show the selected items when the combo is closed

You could combine the two? Use the icons for display, but the popup for selecting them.

Another way of solving this is to display what is selected as the text of the ComboBox like "'Normal Window' selected" or "5 selected". Here is roughly how this can be done: https://stackoverflow.com/questions/47575880/qcombobox-set-title-text-regardless-of-items (if you copy any code from this the file would need to be GPL 3 as part of a CC-BY-SA 4.0 one-way conversion AFAIK)

In T12729#224611, @iasensio wrote:

Here I feel that the non-flat buttons can give a more "busy" feeling when close to the KCM buttons, while flat ones make it more clean, but I'm not strong against it:

I agree but I think going with what is consistent is more important here.

This is also why I agree with broulik that switches shouldn't be used here. Furthermore switches tend to be used to communicate changes that are instantly applied which isn't really the case here.

I've just implemented the display text for the combo box to show the number of items selected (or the corresponding text if only one is selected) as you suggested.

Anyway, I still think that the UX is more annoying and less informative at a glance than using the array of buttons.

About the other smaller questions, you've both convinced me. It's better to get consistent now, and maybe discuss it later on a more global scale.

Anyway, I still think that the UX is more annoying and less informative at a glance than using the array of buttons.

Well, the problem is that we can't expect users to be able to identify window types by their icons. Maybe if we had icons as recognisable as for the desktop, the normal and the dialog window for all of them it would be alright but even then users might still want to hover over some of the other icons to be sure they selected the right one(s). This is especially difficult because some of the window types can seem similar like Dialog and Utility or Dock, Toolbar, Torn-Off Menu and Standalone Menubar.
There really needs to be a list with the actual names of the window types somewhere IMO. And while I don't think this ComboBox will win any design awards I do think it works well enough. There might still be a better way of doing this though.
I really like how easily the window type condition can be read when only one type is selected. I hope that covers the vast majority of use cases.

Looking at some of the rules, I see that the language seems very high. Surely a user that spends time on this KCM would have some understanding of what they are getting into. I would suggest bringing the language for labels closer to the one used for KCM options, where we make it conversational, inviting action. For example,

Under Window Matching:

Description

Use:

Add description
Enter description

Under

Window Types [All Selected]

Maybe use

Apply to [all selected] window types