Review Request 128417: Retain original task button sort order when in manual sort mode and plasmashell restarts.

Eike Hein hein at kde.org
Sun Jul 10 23:45:14 UTC 2016



> On July 10, 2016, 6:42 a.m., Eike Hein wrote:
> > New features to libtaskmanager should be implemented on Wayland first. Otherwise they increase the porting effort, which I'm not willing to do (i.e. I'm not going to take any drive-by feature contributions that leave me to figuring out how to make it work on Wayland later).
> 
> Xuân Baldauf wrote:
>     The feature of the current patch works both under X11 and under Wayland (tested with kwin5-5.7.0 based on wayland-1.11.0). No code change (e.g. in file "waylandtasksmodel.cpp") was necessary on the Wayland side to achieve the desired behavior.

Is that guaranteed in the spec, or does it work by accident? "By accident" isn't good enough - it needs to be a spec guarantee or explicit code. If it's as important as you say it is, it needs to be documented and testable behavior


The diff also contains references to a loftier spec that creates a porting pressure vector. That needs fleshing out or removal.

And there is a UX problem: TasksModel starts out in SortAlpha and then is switched to SortManual by a call to setSortMode. Switching from a different sort mode to SortManual is supposed not to reorder tasks, but seed the map from the the existing sorting, so the user can opt into manual from the applet config dialog and then move tasks from their current location. This patch will make this experience worse by reordering window tasks by creation order. I care more about that case than plasmashell crashing, since at that point things have gone wrong anyway, and crashes are to be fixed.

Further, it doesn't sort in startup tasks  correctly, making things even more arbitrary, even though it's available. TasksModel is supposed to abstract over all task types correctly; doing major business logic in a source model is conceptually wrong. Sorting is also done in TasksModel, not the intentionally simple, data-providing source models. A better appoach to this would be a CreationTimestamp data role in the source models used to seed the sort map - though that by itself isn't enough to address the above UX problem.


- Eike


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128417/#review97248
-----------------------------------------------------------


On July 10, 2016, 12:15 a.m., Xuân Baldauf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128417/
> -----------------------------------------------------------
> 
> (Updated July 10, 2016, 12:15 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> This is the first step towards not loosing the sort order of the task buttons in the task bar, even when plasmashell crashes and the user has a specific manual sort order. For now, we use the sort order imposed by the creation time (so the user can affect the sort order simply by ordering the creation of the windows).
> 
> Once this patch is successful, we can expand sort order storage by storing the manual sort order offset in the X window properties each time that offset changes. This will make plasmashell crashes less damaging to a user who prefers a certain task button sort order (for the duration of the X session).
> 
> 
> Diffs
> -----
> 
>   libtaskmanager/CMakeLists.txt 9c3e15e 
>   libtaskmanager/tasksmodel.cpp bf37042 
>   libtaskmanager/xwindowtasksmodel.h d24a7d5 
>   libtaskmanager/xwindowtasksmodel.cpp c1e9495 
> 
> Diff: https://git.reviewboard.kde.org/r/128417/diff/
> 
> 
> Testing
> -------
> 
> Testing was performed under openSUSE 42.1 against KDE Plasma 5.7
> 
> 
> Thanks,
> 
> Xuân Baldauf
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160710/516ea742/attachment.html>


More information about the Plasma-devel mailing list