Review Request: Plasmate:images can now be added to projects

[Giorgos Tsiapaliwkas]

> On Oct. 25, 2011, 11:02 a.m., Aaron J. Seigo wrote:
> > editors/editpage.cpp, lines 109-110
> > <http://git.reviewboard.kde.org/r/102290/diff/3/?file=39420#file39420line109>
> >
> >     such a check is necessary indeed .. however, i believe KIO::CopyJob provides this internally already. have you tested this?

i replaced KIO::Overwrite with KIO::HideProgressInfo,and the issue was solved


> On Oct. 25, 2011, 11:02 a.m., Aaron J. Seigo wrote:
> > editors/imageviewer/imageviewer.cpp, lines 39-42
> > <http://git.reviewboard.kde.org/r/102290/diff/3/?file=39422#file39422line39>
> >
> >     this will not work as widget is created on the stack, so as soon the if statement is done it will be automatically deleted.

fixed


> On Oct. 25, 2011, 11:02 a.m., Aaron J. Seigo wrote:
> > editors/imageviewer/imageviewer.cpp, line 36
> > <http://git.reviewboard.kde.org/r/102290/diff/3/?file=39422#file39422line36>
> >
> >     this is going to cause problems on re-use. QWidget does not let you set a layout on a widget if it already has one.
> >     
> >     i really, really recommend creating the layout in the consructor, adding a QSvgWidget* and a QLabel* member to this class and using those in setImage, e.g.:'
> >     
> >     if (..svg...) {
> >          delete m_label;
> >          if (!m_svgWidget) {
> >               ... create svg widget  . add it to the layout() .
> >          }
> >         m_svgWidget->load(m_image);
> >     } else {
> >         delete m_svgWidget;
> >         if (!m_label) {
> >     ...etc
> >

ISSUE:In order to create the layout in the constructor i had to newing the QLabel* and QSVGWidget* in the ctor....


- Giorgos


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


On Oct. 20, 2011, 8:49 a.m., Giorgos Tsiapaliwkas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102290/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2011, 8:49 a.m.)
> 
> 
> Review request for Plasma and Aaron J. Seigo.
> 
> 
> Description
> -------
> 
> hello,
> 
> without this patch a user cannot add an image with plasmate.
> 
> reproduce,go to files-images-new,the plasmate will open a text editor instead of a dialog,which(the dialog) is able to open an image.
> 
> With the patch a dialog will open asking for an image.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt f3f32a9 
>   editors/editpage.h 7b5dca3 
>   editors/editpage.cpp d4b0082 
>   editors/imageviewer/imageviewer.h e69de29 
>   editors/imageviewer/imageviewer.cpp e69de29 
>   main.cpp 66a7cd8 
>   mainwindow.h 1b1c2a2 
>   mainwindow.cpp 2fa2742 
>   packagemodel.cpp 8c0907a 
> 
> Diff: http://git.reviewboard.kde.org/r/102290/diff/diff
> 
> 
> Testing
> -------
> 
> the patch is not ready yet,i have noted some questions.
> Also the plasmate tries to open the image with a text editor.This have to be fixed,but how?Should we make plasmate able to preview images?
> 
> In addition,when you add something in the list of files(using the different options provided by the files qdockwidget) it names it as "new".This has to be fixed and the plasmate has to show the real name of the file.(different request,i just want an approval for this patch).
> 
> 
> Thanks,
> 
> Giorgos Tsiapaliwkas
> 
>

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