----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103830/#review10265 -----------------------------------------------------------
i really don't like this change because it puts in a hardcoded assumption about folderview into an independent component. is folderview the only applet/containment that should behave this way? probably not. what if we replace folderview someday with something else? is kickoff the only place from which you can add an item to a folderview? it's just not generic enough, and that "no assumptions" approach is something that we stick to in plasma which allows us to easily pull components out, replace them with others, etc. we already have this solved when there is a drag and drop event: each applet handles its own drop events and does what is necessary. on the other hand, i also don't want to see an "addUrl" type method to Applet since that is going to be irrelevant to the vast majority of Applets and itself contains some assumptions. this calls for a better solution that can be implemented in the target Applet and which doesn't result in assumptions about what the source or targets are. plasma/desktop/applets/kickoff/ui/contextmenufactory.cpp <http://git.reviewboard.kde.org/r/103830/#comment8447> i would turn this into a an "else if" and get rid of one level of indentation - Aaron J. Seigo On Jan. 31, 2012, 3:45 p.m., Ignat Semenov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/103830/ > ----------------------------------------------------------- > > (Updated Jan. 31, 2012, 3:45 p.m.) > > > Review request for Plasma and Aaron J. Seigo. > > > Description > ------- > > Currently, right-clicking a Kickoff/Classical application launcher entry and > selectiong "Add to Desktop" puts an icon applet on the desktop. However, if > the desktop is set to Folderview, it is more consistent to add a link to the > currently displayed folder. This patch cheks if the "folderview" plugin is > used in the desktop containment and performs a KIO::link() if that's the case. > > > Diffs > ----- > > plasma/desktop/applets/kickoff/ui/contextmenufactory.cpp 8db9655 > > Diff: http://git.reviewboard.kde.org/r/103830/diff/diff > > > Testing > ------- > > Tested, works. > > > Thanks, > > Ignat Semenov > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
