> 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

Reply via email to