This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104025/

On March 1st, 2012, 11:19 a.m., Hans Chen wrote:

(Disclaimer: I'm just a user, hope it's OK I comment on the UI. :)

Personally I think the screenshot looks a bit daunting because these comboboxes seem to represent a lot of options. How many items are there in each dropdown list? If there are only two, I would personally prefer checkboxes like in the taskmanager widget settings, e.g.

Only show windows from the current application [ ]
Only show windows from the current screen [ ]
Only show windows from the current desktop [ ]
Only show windows from the current activity [ ]
Only show windows that are minimized [ ]
Include desktop in the list [ ]

In my opinion it makes it clearer what each option does, you only have two choices for each option (yes/no) and it makes it consistent with Plasma (note that the taskmanager uses "tasks" instead of "windows").

I noticed that you wrote "you [Martin] requested all checks to be combo boxes" - was this what you referred to?

On March 1st, 2012, 11:25 a.m., Martin Gräßlin wrote:

the user interface has to be changed, that is quite clear. But it is unrelated to the refactoring changes in fact.

What I want to see is making the layout selection the most prominent part of the UI and "hide" the settings. For the settings I'm thinking of an approach like the mouse actions for Plasma Desktop. Using a button to "Add another restriction" or something like that. Would make the view much less cluttered.

On March 1st, 2012, 11:43 a.m., Stefano Avallone wrote:

As Martin said, the UI has to be redesigned. To reply to Hans' question: the Applications, Minimized and MultiScreen combo boxes have three options (please have a look at the enum types in the new version of tabboxconfig.h)

To Martin: by the week end, I'll try to upload a new version of the diff addressing your comment above and rebased against current master
Thank you both for your replies!

@Martin: I had a similar thought but would prefer one that requires as little "work" from the user as possible.

@Stefano: I see, that makes it more complicated.

Since the UI is unrelated to this patch I'll add my comments regarding it somewhere else, possibly the KWin mailing list.

- Hans


On February 21st, 2012, 3:42 p.m., Stefano Avallone wrote:

Review request for kwin and Martin Gräßlin.
By Stefano Avallone.

Updated Feb. 21, 2012, 3:42 p.m.

Description 

This patch refactors the checks for inclusion of TabBox clients in the client list. All the checks have been implemented as dropdowns. Quoting Martin, the checks are:

* Desktop: Windows From All Desktops, Windows From Current Desktop
* Activities: Windows From All Activities, Windows From Current Activity
* Applications: All Windows From All Applications, Only one Window per
Application, Only all Windows of Current Application
* Minimized: Ignore, Minimized windows only, Exclude Minimized Windows
* Desktop: Don't Show Desktop, Show Desktop to minimize all windows
* Multi Screen: Ignore, Only Windows from current screen, Exclude windows from
current screen

Each of such checks has been implemented as a private method of TabBoxHandlerImpl. They are called by TabBoxHandlerImpl::clientToAddToList, while ClientModel::createClientList only handles the switching mode (FocusChain/StackingOrder) and whether a desktop client should be added to the list to minimize all the clients. In TabBoxHandlerImpl::clientToAddToList, I left the following line:

addClient = addClient && current->wantsTabFocus() && !current->skipSwitcher();

as I am not sure if it is needed or not.

The UI is a bit crowded now, but I would leave a UI refactor for a future commit.

Testing 

Desktop, Minimized, Show desktop: work fine

Multi Screen: could not test, as I do not have a multi screen configuration

Activities: works fine. However, when using the present windows effect as task switcher, the previews of the windows on other activities are not visible. They are actually there, because moving the mouse to the area where they (presumably) are makes the close button appear and allows to activate them. When using a layout based switcher, thumbnails of windows on other activities are correctly shown. So, I guess, it is something unrelated to the checks I made.

Applications: the new case I implemented is "Only all Windows of Current Application" (others work fine). I tested it with KDevelop+About KDE+About Kdevelop, and it works fine. It doesn't work with the three windows opened by Gimp, in the sense that only one window (instead of all of them) is shown. Not sure if my check is incorrect, here.

Diffs

  • kwin/kcmkwin/kwintabbox/main.cpp (3f047d1)
  • kwin/kcmkwin/kwintabbox/main.ui (a316b28)
  • kwin/tabbox/clientmodel.cpp (4ca7fef)
  • kwin/tabbox/declarative.cpp (dd4f242)
  • kwin/tabbox/tabbox.h (6d4e2b2)
  • kwin/tabbox/tabbox.cpp (3f26397)
  • kwin/tabbox/tabboxconfig.h (32facde)
  • kwin/tabbox/tabboxconfig.cpp (9feaa16)
  • kwin/tabbox/tabboxhandler.h (27b475a)

View Diff

Screenshots

Modified UI