Taskmanager with grouping support preview version

[christian mollekopf]

hi, im just going to answer on point where i disagree or have further questions. 

> so could you please commit your code into a branch in KDE's svn? that way
> there's a hope of tracking what's going on and getting involved.
> /branches/work/plasma-groupingtaskbar or something like that would be great.

I have an account now and will upload the files a soon as i have done the basic refactoring according to your comments.



> BasicGroupingStrategy:
>
> * since it's meant to be subclasses, do we need the word "Basic" in it? it
> actually doesn't seem to do any grouping, actually, but i assume that's due to
> the stage of devel it's in... if it's meant to be a shell of a class that is
> meant to be subclassed (i see virtuals =) then it could be
> AbstractGroupingStrategy and the virtuals made pure virtual. personally, i'd
> just call it GroupingStrategy

true...

> * in the taskbar we're usually getting one task at a time from the window
> manager (as windows are added/removed). i don't see any API that easily maps
> an item to an existing group. that seems odd. where does the "which group
> should this task be put in?" code currently exist?

You're right, the task should probably first be passed to the GroupingStrategy which tries to put it into the best TaskGroup or the RootGroup.
The group which receives the task, then tries to group fitting tasks together.

> * what is the purpose of having separate suggesting methods for names, colours
> and icons, other than to go through the list 3 times? i can't think of any
> situation where you'd only want the names or only the colours, or in which
> having the other information would be detrimental. on the flip side, going
> through the list N times is a bit wasteful. not to mention it complicates the
> API =)

Those methods were thought for later suggestions in the GroupEdit window where you
should be able to choose between suggestions freely or enter new names, colors etc.

> * why is it a singleton when there can be N users of a strategy? why is it a
> singleton if it is meant to be subclassed? this really doesn't look like the
> place for a singleton, tbh.




Well, the basicGroupingStrategy wasn't really anything useful...

my idea of the GroupingStrategy is that as soon as a AbstractGroupableItem is added or removed to a TaskGroup,
the TaskGroup passes its children to the GroupingStrategy singleton, and the singleton returns possible matches which items should group together.
If the match isn't just possible but rather for sure, the Group groups the items according to the GroupSuggestion returned by the GroupingStrategy.
If the match is just a suggestion the Group ignores it. Everytime the user groups tasks manually or renames a group the GroupingStrategy learns from it.

The GroupingStrategy would hold the configuration and various other GroupingStrategies which are derived from AbstractGroupingStrategy and deliver the actual guesses.


> * where will ordering of items be handled? you seem to be assuming a manual
> ordering of things always and that's not what most people want: they want an
> automated ordering (alpha, by desktop, etc). i'm not sure it belongs in
> TaskGroup because all task groups in a given collection should behave
> similarly and alternative strategies would require either putting all possible
> strategies into TaskGroup or making it subclassable. (neither are pretty)

The ordering should probably also be handled by the GroupingStrategy and we could have there a ManualOrdering mode...

> TaskGroup:
>
> * i don't understand the relationship between the grouping strategy and the
> group class. if you have N tasks divided into M groups, what decides what task
> belongs in each group? right now it seems the strategy suggests where they
> should go, but the group decides. what happens in the case of multiple groups
> when a new task comes around?
>
> the way i might go about it is have the strategy manage a collection of groups
> that it then exports. this would be a tree of objects representing the groups
> and their nesting. groups would contain items and be completely stupid about
> the grouping mechanics. that way the relationship between group and strategy
> is obvious, it's clear how multiple strategies would work in parallel, and
> give a sane point for configuration management

I don't get this part, sorry. On what would this tree of groups be based?



> * there are gui concepts mixed into this data class. collapsed/not collapsed
> is not a data concept, it's a visualization concept. all of that logic belongs
> in the UI side.

Thats true, but if i remove it i have to move the whole TaskRMBMenu class out of the lib and move it to
workspace/plasma/applets/tasks/ so i have direct access to the collapsed() function.

> a few implementation comments:
>
> TaskGroup:

> * add(dynamic_cast<AbstractGroupableItem*>(task.data())) -> *never* take the
> data of a TaskPtr and store it somewhere. that defeats the purpose of TaskPtr
> altogether. moreover, you don't need to cast a subclass to a superclass.
> dynamic_cast is for downcasting. in fact, i don't actually see the point of
> the various add() method overrides (for TaskPtr, TaskGroup, etc). they don't
> actually do anything useful.
>
> * ditto for the remove() methods

You're right with the directly inherited classes. Unfortuantely i couldn't find out
if this is also possible with the KSharedPointers. Therefore i had to keep those functions.
It looks like this at the moment:

typedef KSharedPtr<AbstractGroupableItem> AbstractPtr;
typedef KSharedPtr<TaskGroup> GroupPtr; //Task group inherits AbstractGroupableItem

void TaskGroup::add(GroupPtr item)
{
    add(AbstractPtr::dynamicCast(item));
}

void TaskGroup::add(AbstractPtr item)
{
    if (d->m_members.contains(item)) {
        kDebug() << "already in this group";
        return;
    }
    d->m_members.insert(item);
    item->addedToGroup(GroupPtr(this));
    emit itemAdded(item);
}



> > -Save custom order on collapse group
>
> custom order? you mean the order someone has dragged the item into? this is
> not going to work with automated systems, or at least make them very
> complicated, or require being able to switch

Well, then at least in this mode. To me as a user it was almost a "must have" feature to
sort the items as i like but its maybe not that crucial anymore with groups...

> > -It should be possible to have multiple rows and force a certain number of
> > rows or let the LayoutWidget automatically appen and remove rows as needed.
> beware of "a million dialogs" syndrome

This option isn't necessarily available to the user. Its more that the rootgroup can have
several rows while subgroups can be forced to use just one...


> > -Implement Dragging of expanded groups by dragging groupmember outside of
> > group
>
> unless the implementation is more straightforward than the description, this
> could be a problematic feature.

This works actually quite well...

> --
> Aaron J. Seigo
> humru othro a kohnu se
> GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43
>
> KDE core developer sponsored by Trolltech
>


Regards


_________________________________________________________________
Windows Live Spaces - Ihr Leben, Ihr Space. Hier klicken und informieren.
http://get.live.com/spaces/overview
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/panel-devel/attachments/20080707/a110e8fa/attachment-0001.html