> 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
>
> Aaron J. Seigo wrote:
> 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?
QML is intended to provide a simple way to describe GUI and GUI logic. No more.
C++ code provides user's preference about visibility of task: he can want to
see item always or not. But the real responsibility for visibility of task
belongs to GUI logic (QML code). QML code handles changing of status of task,
takes a decision on location (popup/panel) of task. GUI part/QML code is a
representation of tray tasks and it may not take into account user's preference
at all. So, I don't want split GUI logic, I don't want duplicate the same code
in StatusNotifierItem and WidgetItem. Moreover StatusNotifierItem is
implemented in QML.
I think C++ should provide user's preference and signal if he changes his
preference.
- Dmitry
-----------------------------------------------------------
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
