Review Request: load the images for the themes correctly

Aaron J. Seigo aseigo at kde.org
Tue Nov 6 13:28:41 UTC 2012



> On Nov. 2, 2012, 10:20 a.m., Aaron J. Seigo wrote:
> > plasmate/packagemodel.cpp, lines 546-551
> > <http://git.reviewboard.kde.org/r/106680/diff/4/?file=93274#file93274line546>
> >
> >     this looks wrong; in the Package definition there is an entry for "colors". even if it is not created by default it should still show up in the model. the whole point of this model, after all, is to have a generic way to represent a Plasma::Package.
> >     
> >     same with the checks for Plasma/Theme in the data() method. none of these should be necessary.
> 
> Giorgos Tsiapaliokas wrote:
>     I don't understand.
>     
>     >this looks wrong; in the Package definition there is an entry for "colors". even if it is not created by default it should still show up in the model
>     
>     with the above it will always show the colors in the ui

hardcoding package-specific data in the model is wrong. the point of having Package is to have a generic way to describe packages. if we then have to put package-specific hacks into our code, it is failing to do this :)

so ... what needs to be done is to find a way to show the colors entry (or any similar top-level item) when a Package is loaded into the model.


- Aaron J.


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


On Nov. 5, 2012, 5:51 p.m., Giorgos Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106680/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2012, 5:51 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> create a new theme package->click on the new
> 
> a file dialog should appear but instead a simple edit box appears requesting a new filename.
> 
> This patch solves the issue
> 
> 
> Diffs
> -----
> 
>   plasmate/editors/editpage.h 5cb3ea6 
>   plasmate/editors/editpage.cpp 7e82ff2 
>   plasmate/packagemodel.h efa3001 
>   plasmate/packagemodel.cpp 9eb0914 
> 
> Diff: http://git.reviewboard.kde.org/r/106680/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Giorgos Tsiapaliokas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20121106/b3bd4e5e/attachment-0001.html>


More information about the Plasma-devel mailing list