> 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? > > Dmitry Ashkadov wrote: > 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.
I think Plasmoid class may be merged to Applet. If formFactor and location properties were implemented in Plasma::Applet, I would remove them from Plasmoid class during merging. - Dmitry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107160/#review21486 ----------------------------------------------------------- On Nov. 7, 2012, 3:20 p.m., Dmitry Ashkadov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107160/ > ----------------------------------------------------------- > > (Updated Nov. 7, 2012, 3:20 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