<html>
<head>
<style>
.hmmessage P
{
margin:0px;
padding:0px
}
body.hmmessage
{
FONT-SIZE: 10pt;
FONT-FAMILY:Tahoma
}
</style>
</head>
<body class='hmmessage'>
hi, im just going to answer on point where i disagree or have further questions. <br><br>> so could you please commit your code into a branch in KDE's svn? that way<br>> there's a hope of tracking what's going on and getting involved.<br>> /branches/work/plasma-groupingtaskbar or something like that would be great.<br><br>I have an account now and will upload the files a soon as i have done the basic refactoring according to your comments.<br><br><br><br>> BasicGroupingStrategy:<br>><br>> * since it's meant to be subclasses, do we need the word "Basic" in it? it<br>> actually doesn't seem to do any grouping, actually, but i assume that's due to<br>> the stage of devel it's in... if it's meant to be a shell of a class that is<br>> meant to be subclassed (i see virtuals =) then it could be<br>> AbstractGroupingStrategy and the virtuals made pure virtual. personally, i'd<br>> just call it GroupingStrategy<br><br>true...<br><br>> * in the taskbar we're usually getting one task at a time from the window<br>> manager (as windows are added/removed). i don't see any API that easily maps<br>> an item to an existing group. that seems odd. where does the "which group<br>> should this task be put in?" code currently exist?<br><br>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.<br>The group which receives the task, then tries to group fitting tasks together.<br><br>> * what is the purpose of having separate suggesting methods for names, colours<br>> and icons, other than to go through the list 3 times? i can't think of any<br>> situation where you'd only want the names or only the colours, or in which<br>> having the other information would be detrimental. on the flip side, going<br>> through the list N times is a bit wasteful. not to mention it complicates the<br>> API =)<br><br>Those methods were thought for later suggestions in the GroupEdit window where you<br>should be able to choose between suggestions freely or enter new names, colors etc.<br><br>> * why is it a singleton when there can be N users of a strategy? why is it a<br>> singleton if it is meant to be subclassed? this really doesn't look like the<br>> place for a singleton, tbh.<br><br><br><br><br>Well, the basicGroupingStrategy wasn't really anything useful...<br><br>my idea of the GroupingStrategy is that as soon as a AbstractGroupableItem is added or removed to a TaskGroup,<br>the TaskGroup passes its children to the GroupingStrategy singleton, and the singleton returns possible matches which items should group together.<br>If the match isn't just possible but rather for sure, the Group groups the items according to the GroupSuggestion returned by the GroupingStrategy.<br>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.<br><br>The GroupingStrategy would hold the configuration and various other GroupingStrategies which are derived from AbstractGroupingStrategy and deliver the actual guesses.<br><br><br>> * where will ordering of items be handled? you seem to be assuming a manual<br>> ordering of things always and that's not what most people want: they want an<br>> automated ordering (alpha, by desktop, etc). i'm not sure it belongs in<br>> TaskGroup because all task groups in a given collection should behave<br>> similarly and alternative strategies would require either putting all possible<br>> strategies into TaskGroup or making it subclassable. (neither are pretty)<br><br>The ordering should probably also be handled by the GroupingStrategy and we could have there a ManualOrdering mode...<br><br>> TaskGroup:<br>><br>> * i don't understand the relationship between the grouping strategy and the<br>> group class. if you have N tasks divided into M groups, what decides what task<br>> belongs in each group? right now it seems the strategy suggests where they<br>> should go, but the group decides. what happens in the case of multiple groups<br>> when a new task comes around?<br>><br>> the way i might go about it is have the strategy manage a collection of groups<br>> that it then exports. this would be a tree of objects representing the groups<br>> and their nesting. groups would contain items and be completely stupid about<br>> the grouping mechanics. that way the relationship between group and strategy<br>> is obvious, it's clear how multiple strategies would work in parallel, and<br>> give a sane point for configuration management<br><br>I don't get this part, sorry. On what would this tree of groups be based?<br><br><br><br>> * there are gui concepts mixed into this data class. collapsed/not collapsed<br>> is not a data concept, it's a visualization concept. all of that logic belongs<br>> in the UI side.<br><br>Thats true, but if i remove it i have to move the whole TaskRMBMenu class out of the lib and move it to<br>workspace/plasma/applets/tasks/ so i have direct access to the collapsed() function.<br><br>> a few implementation comments:<br>><br>> TaskGroup:<br><br>> * add(dynamic_cast<AbstractGroupableItem*>(task.data())) -> *never* take the<br>> data of a TaskPtr and store it somewhere. that defeats the purpose of TaskPtr<br>> altogether. moreover, you don't need to cast a subclass to a superclass.<br>> dynamic_cast is for downcasting. in fact, i don't actually see the point of<br>> the various add() method overrides (for TaskPtr, TaskGroup, etc). they don't<br>> actually do anything useful.<br>><br>> * ditto for the remove() methods<br><br>You're right with the directly inherited classes. Unfortuantely i couldn't find out<br>if this is also possible with the KSharedPointers. Therefore i had to keep those functions.<br>It looks like this at the moment:<br><br>typedef KSharedPtr<AbstractGroupableItem> AbstractPtr;<br>typedef KSharedPtr<TaskGroup> GroupPtr; //Task group inherits AbstractGroupableItem<br><br>void TaskGroup::add(GroupPtr item)<br>{<br> add(AbstractPtr::dynamicCast(item));<br>}<br><br>void TaskGroup::add(AbstractPtr item)<br>{<br> if (d->m_members.contains(item)) {<br> kDebug() << "already in this group";<br> return;<br> }<br> d->m_members.insert(item);<br> item->addedToGroup(GroupPtr(this));<br> emit itemAdded(item);<br>}<br><br><br><br>> > -Save custom order on collapse group<br>><br>> custom order? you mean the order someone has dragged the item into? this is<br>> not going to work with automated systems, or at least make them very<br>> complicated, or require being able to switch<br><br>Well, then at least in this mode. To me as a user it was almost a "must have" feature to<br>sort the items as i like but its maybe not that crucial anymore with groups...<br><br>> > -It should be possible to have multiple rows and force a certain number of<br>> > rows or let the LayoutWidget automatically appen and remove rows as needed.<br>> beware of "a million dialogs" syndrome<br><br>This option isn't necessarily available to the user. Its more that the rootgroup can have<br>several rows while subgroups can be forced to use just one...<br><br><br>> > -Implement Dragging of expanded groups by dragging groupmember outside of<br>> > group<br>><br>> unless the implementation is more straightforward than the description, this<br>> could be a problematic feature.<br><br>This works actually quite well...<br><br>> --<br>> Aaron J. Seigo<br>> humru othro a kohnu se<br>> GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43<br>><br>> KDE core developer sponsored by Trolltech<br>><br><br><br>Regards<br><br><br /><hr />Bloggen, posten und Dateien freigeben für Freunde und Familie - jetzt noch einfacher mit Windows Live Spaces! <a href='http://get.live.com/messenger/overview' target='_new'>Hier klicken!</a></body>
</html>