Review Request: load the images for the themes correctly

[Giorgos Tsiapaliokas]

> On Nov. 29, 2012, 1:53 p.m., Aaron J. Seigo wrote:
> > plasmate/packagemodel.cpp, line 544
> > <http://git.reviewboard.kde.org/r/106680/diff/5/?file=93807#file93807line544>
> >
> >     "Plasma/Theme" should not appear in this file.
> 
> Giorgos Tsiapaliokas wrote:
>     the "colors" file isn't a requiredDirectory nor a requiredFile, so If I don't add it by hand in the m_topEntries
>     it won't be visible and also we want the "colors" file in the m_topEntries only when the package is a theme.
>     No? What am I missing?
> 
> Aaron J. Seigo wrote:
>     perhaps add an exception for top level non-required files, so that everything in the top level, required or not, is shown.

I can't find a way (I don't think that there is one) in order to specify which files are top level ones.

Before the refactor of the packagemodel, the model was doing this:
* find all the files(required or not)
* check if you can give them a name and call them namedFiles(remove the ones which have a name from the hash)
* everything that has been left in the hash add it as a top level item

Now with the refactor of the packagemodel, it does:
* put the required files in a hash and all(including the required ones) the files in another one
** We need the second hash with all the files, because if the project already exists, we want to
add in the ui(with names) all the existing files not just the required ones.
* add all the required files in the ui and give them a name (namedFiles)
* if there are requiredFiles still in the hash add them in the ui.

I see your point that adding a hard coded key can make plasmate fail at some point,
but those keys doesn't change often(actually they don't change at all. No?) so we are fine(not ok).
Even if plasmate fails, in the -worst/and only- case scenario the user won't be able see/modify the "colors" file


> On Nov. 29, 2012, 1:53 p.m., Aaron J. Seigo wrote:
> > plasmate/packagemodel.cpp, lines 196-205
> > <http://git.reviewboard.kde.org/r/106680/diff/5/?file=93807#file93807line196>
> >
> >     why not add "images", "config" and "animations" to m_themeImageDialogOptions and rename it to m_dialogOptions
> 
> Giorgos Tsiapaliokas wrote:
>     I have put  "images" and "config" into the hash, but how can it be done for "animations" too?
>     
>     > else if(!qstrcmp(key, "animations") && packageType() != "Plasma/Theme")
>     we check the key and the packageType, so we can't use the hash here. No?
> 
> Aaron J. Seigo wrote:
>     i guess the real question here is why they are using such different dialogs :) could they be made to use the same dialog? if not, could the switching between the different dialogs be done outside of the PackageModel?

there are two files with the name "animations" the one is in the plasmoidpackage which(the file) is a normal file
and the second one is in the themepackage which is an image. So we need two different dialogs.
I guess we can decide for the proper dialog in the editpage.cpp but still we would have to do the 

>if (somethingPackageType() != "Plasma/Theme")

plus that in the editpage.cpp we don't have access to this information, so we have to write more code. :)
One possible solution could be to not have two "animations" keys. We could have a "theme_animations" and a "plasmoid_animations".


- Giorgos


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


On Dec. 7, 2012, 5:58 p.m., Giorgos Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106680/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2012, 5:58 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/20121216/f47807f8/attachment.html>