Review Request: Let Kickoff and SimpleLauncher widgets preserve their settings after menu style change

[Darío Andrés]

> On 2010-02-15 02:44:08, Aaron Seigo wrote:
> > this is an improvement functionality wise; i think it would be nicer if the applet wasn't destroyed and recreated but simple switch the UI; e.g. if the popup widget was swapped between being the kickoff ui and the QMenu. this would mean some juggling in createConfigurationInterface to create the proper interface, but it is doable. in any case, if you don't feel like doing that work (and i'd completely understand) then at least this patch improves the situation even if it is hackish.

Yes, merging both plasma widgets will be the best but it is too much work for me at the moment. We could try to "polish this hack" to at least get something better. 
Thanks for the review.


> On 2010-02-15 02:44:08, Aaron Seigo wrote:
> > svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/plasma/desktop/applets/kickoff/applet/applet.cpp, line 143
> > <http://reviewboard.kde.org/r/2923/diff/2/?file=19427#file19427line143>
> >
> >     should probably just be a call to configChanged()

Calling simpleLauncher->configChanged() would cause a config reload? which local Applet function is responsible of reloading the config ? init() ?
May be I'm missing something


- Darío


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2923/#review4154
-----------------------------------------------------------


On 2010-02-15 02:28:15, Darío Andrés wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2923/
> -----------------------------------------------------------
> 
> (Updated 2010-02-15 02:28:15)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Because of the current implementation (kickoff and simplelauncher are two separate applets), switching from one launcher style to another is just a "create a new launcher using the other style and delete me!". This causes all the configuration values to be lost from change to change (including the Icon which is a common setting between the two styles)
> In the past I have implemented code to preserve the launcher shortcut; however all the other settings are still lost.
> 
> My patch provides a way to save the configuration values (both config() and globalConfig()) from one applet to another; and reload the applet settings to use the new config values.
> 
> The code could be a bit hackish (passing KConfigGroups and using copyTo()). 
> 
> I also implemented a delayedConfigLoad var, to not load the settings two times (the first, when the applet is created and the init() function is called; and the second one, when the previously stored values are copied...) [Note, this was only done on the SimpleLauncher, as the delayed config load on Kickoff caused the widget to be "iconized" even on the default containment of PlasmoidViewer)
> 
> This shouldn't cause noticeable delays or artifacts as reloadConfigValues() is called inmediatly after the applet creation.
> 
> Hopefully the APIDOX and the comments are self-explanatory :)
> 
> [ If somebody knows how to give a default configuration file; or how to copy the configuration file before the init() call, please tell me and I will improve the patch]
> 
> 
> This addresses bug 192836.
>     https://bugs.kde.org/show_bug.cgi?id=192836
> 
> 
> Diffs
> -----
> 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/plasma/desktop/applets/kickoff/applet/applet.h 1088295 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/plasma/desktop/applets/kickoff/applet/applet.cpp 1088295 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.h 1088295 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp 1088295 
> 
> Diff: http://reviewboard.kde.org/r/2923/diff
> 
> 
> Testing
> -------
> 
> All the config values (from both styles) are preserved after switching back and forward a couple of times.
> (The SimpleLauncher constructor may be some review, as I changed the way the applet's args var is handled... but I think it is ok)
> Also, someone needs to check if the delayedConfigLoad could cause some issue (I didn't notice anyone)
> 
> 
> Thanks,
> 
> Darío
> 
>