> On Nov. 6, 2012, 1:39 p.m., Aaron J. Seigo wrote:
> > one small issue with where hiding status is kept.
> >
> > it may be useful to keep in mind the difference between the data and the
> > visualization -> anything that the use can see (e.g. whether an icon is
> > hidden or not) is visualization and must not be in the data classes; but
> > any data that is used to populate the visualization (e.g. the current
> > attention status of an item) should be handled by the data classes. this
> > data/visualization split is common in plasma, and once one gets used to it
> > things become easier :)
> >
> > anyways.. this one small change of moving the hiding status from Task to
> > WidgetItem and this can be pushed into master :)
> >
> > thanks ...
>
> Dmitry Ashkadov wrote:
> Moving hiding property from Task to WidgetItem won't work, because it is
> used in QML to determine the location/area where the task will be placed
> (popup dialog or panel).
>
> Dmitry Ashkadov wrote:
> And this is a reason to create UiTask for visualization: Task - data,
> UiTask - visualization
WidgetItem is that same thing, however.
right now in the code there is the getLocationForTask function. it does this:
var loc = getDefaultLocationForTask(task)
if (loc === JS.LOCATION_TRAY && task.typeId ==
JS.TASK_NOTIFICATIONS_TYPEID)
return JS.LOCATION_NOTIFICATION // redefine location for
notifications applet
return loc
getDefaultLocationForTask is:
/// Returns location depending on status and hide state of task
function getDefaultLocationForTask(task) {
if (task.status === TaskStatusAttention || task.hideState ===
TaskHideStateShown) return JS.LOCATION_TRAY
if (task.hideState === TaskHideStateHidden || (task.status !==
TaskStatusActive && task.status !== TaskStatusUnknown)) {
return JS.LOCATION_POPUP
}
return JS.LOCATION_TRAY
}
in all cases where this is used, the code already has a WidgetItem or a
StatusNotifierItem. so getDefaultLocationForTask could just as easily take a
WidgetItem or a StatusNotifierItem and these (C++) classes could provide the
same function as Task::visibilityPreference().
the onVisibilityPreference signal is also used only in WidgetItem and
StatusNotifierItem .. so they could just as well do that internally in the C++.
i don't see anywhere that needs this information that does not already have a
WidgetItem or a StatusNotifierItem?
- Aaron J.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107160/#review21486
-----------------------------------------------------------
On Nov. 2, 2012, 12:40 p.m., Dmitry Ashkadov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107160/
> -----------------------------------------------------------
>
> (Updated Nov. 2, 2012, 12:40 p.m.)
>
>
> Review request for Plasma, Aaron J. Seigo and Marco Martin.
>
>
> Description
> -------
>
> Aaron has noticed in his mail to plasma-devel that the system tray requires
> some refactoring. This is a first step of refactoring. If it is approved I'll
> continue work on system tray.
>
>
> Diffs
> -----
>
> plasma/generic/applets/systemtray/CMakeLists.txt d3478a8
> plasma/generic/applets/systemtray/core/task.h 66bf6e1
> plasma/generic/applets/systemtray/core/task.cpp 8754785
> plasma/generic/applets/systemtray/package/contents/code/main.js 10518cd
> plasma/generic/applets/systemtray/package/contents/ui/IconsList.qml f251cc5
>
> plasma/generic/applets/systemtray/package/contents/ui/StatusNotifierItem.qml
> 27d476a
> plasma/generic/applets/systemtray/package/contents/ui/main.qml f80abc7
>
> plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtraytask.h
> 34aae74
> plasma/generic/applets/systemtray/protocols/fdo/fdotask.h 85a9ec5
> plasma/generic/applets/systemtray/protocols/plasmoid/plasmoidtask.h 1913986
> plasma/generic/applets/systemtray/ui/applet.h bab8d9c
> plasma/generic/applets/systemtray/ui/applet.cpp 41efb10
> plasma/generic/applets/systemtray/ui/mouseredirectarea.h 91c40c0
> plasma/generic/applets/systemtray/ui/mouseredirectarea.cpp aac2a53
> plasma/generic/applets/systemtray/ui/plasmoid.h 01a7c5b
> plasma/generic/applets/systemtray/ui/plasmoid.cpp d3e1937
> plasma/generic/applets/systemtray/ui/taskspool.h 4b5bcd4
> plasma/generic/applets/systemtray/ui/taskspool.cpp df39e3d
> plasma/generic/applets/systemtray/ui/uitask.h 7b20bde
> plasma/generic/applets/systemtray/ui/uitask.cpp 2a15800
> plasma/generic/applets/systemtray/ui/widgetitem.h 40aa92d
> plasma/generic/applets/systemtray/ui/widgetitem.cpp 9d2c622
>
> Diff: http://git.reviewboard.kde.org/r/107160/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Ashkadov
>
>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
