[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Review Request 119658: [WIP] Support per-activity favourites in Kicker
From: Ivan Čukić <ivan.cukic () kde ! org>
Date: 2014-08-08 9:33:52
Message-ID: 20140808093352.20974.99714 () probe ! kde ! org
[Download RAW message or body]
--===============5042159889112187698==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119658/
-----------------------------------------------------------
Review request for Plasma, Eike Hein and Marco Martin.
Repository: plasma-desktop
Description
-------
Basic changes to support KActivities.ResourceModel-based favourites in Kicker.
What works:
- Favourites are shown
- They can be added and removed
- Shared with other launchers (currently kickoff)
What does not (request for comments):
- launching the favourites
KActivities.ResourceModel does not have a method to open an URL, while Kicker \
relies on the 'trigger' method. The important question to discuss is whether it \
should know that at all, or it should leave launching to the client application (like \
kickoff currently does). From my POV, the model *needs* to only provide the URL, and \
the application should know how to open the URL it provided to the model in the first \
place. It *could* have a (convenience) method to launch the URL, but there will be a \
lot of URLs that it can not understand (like akonadi:something, etc.). The \
convenience method would be useful in a few places, not only Kicker.
- drag'n'drop rearanging does not work
KActivities.ResourceModel is based on QSortFilterProxyModel. This means that \
whenever an item is moved from one place to another, the model reloads itself (at \
least, the client sees it as a reload). This QSFPM behaviour collides with the d'n'd \
implementation in Kicker (as opposed to Kickoff) because it does the d'n'd by moving \
an item one place at a time, instead of moving it directly to the destination. That \
is, if an item is moved from 2nd to 5th position, Kicker will move it 2nd->3rd, \
3rd->4th, 4th->5th. The model will invalidate the dragged item on the first step, and \
other steps will not work.
I need ideas and opinions for these issues.
Cheers!
Diffs
-----
applets/kicker/package/contents/ui/SideBarSection.qml 0411545
applets/kicker/plugin/favoritesmodel.h 342d3d1
applets/kicker/plugin/favoritesmodel.cpp 3494a65
Diff: https://git.reviewboard.kde.org/r/119658/diff/
Testing
-------
Thanks,
Ivan Čukić
--===============5042159889112187698==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119658/">https://git.reviewboard.kde.org/r/119658/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Plasma, Eike Hein and Marco Martin.</div>
<div>By Ivan Čukić.</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-desktop
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Basic changes to support \
KActivities.ResourceModel-based favourites in Kicker.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What \
works:<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" />
- Favourites are shown<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" />
- They can be added and removed<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" />
- Shared with other launchers (currently kickoff)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">What does not (request for comments):<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
- launching the favourites</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">KActivities.ResourceModel does not have a method to \
open an URL, while Kicker relies on the 'trigger' method. The important question to \
discuss is whether it should know that at all, or it should leave launching to the \
client application (like kickoff currently does).<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> \
From my POV, the model <em style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;">needs</em> to only provide the URL, and \
the application should know how to open the URL it provided to the model in the first \
place. It <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">could</em> have a (convenience) method to launch the \
URL, but there will be a lot of URLs that it can not understand (like \
akonadi:something, etc.). The convenience method would be useful in a few places, not \
only Kicker.</p> <ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 \
1em;line-height: inherit;white-space: normal;"> <li style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;">drag'n'drop rearanging \
does not work</li> </ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">KActivities.ResourceModel is based on \
QSortFilterProxyModel. This means that whenever an item is moved from one place to \
another, the model reloads itself (at least, the client sees it as a reload).<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" /> This QSFPM behaviour collides with the d'n'd implementation in Kicker \
(as opposed to Kickoff) because it does the d'n'd by moving an item one place at a \
time, instead of moving it directly to the destination. That is, if an item is moved \
from 2nd to 5th position, Kicker will move it 2nd->3rd, 3rd->4th, 4th->5th. \
The model will invalidate the dragged item on the first step, and other steps will \
not work.</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I need ideas and opinions for these issues.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Cheers!</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>applets/kicker/package/contents/ui/SideBarSection.qml <span style="color: \
grey">(0411545)</span></li>
<li>applets/kicker/plugin/favoritesmodel.h <span style="color: \
grey">(342d3d1)</span></li>
<li>applets/kicker/plugin/favoritesmodel.cpp <span style="color: \
grey">(3494a65)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119658/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============5042159889112187698==--
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic