Review Request: Plasmate: Add KWin Scripting support to konsolepreviewer.

[Sebastian Kügler]

On Friday, August 24, 2012 10:23:07 Antonis Tsiapaliokas wrote:
> > On Aug. 23, 2012, 10:19 p.m., Sebastian Kügler wrote:
> > > mainwindow.cpp, line 261
> > > <http://git.reviewboard.kde.org/r/106118/diff/1/?file=79947#file79947lin
> > > e261>> >
> > >     I don't really like this deviation in behavior, mainly because I
> > >don't see a reason for it. 
> > >     I think the behaviour of opening the konsole from the previewer is
> > >not ideal anyway, since that's a very unusual way to trigger a change in
> > >the main UI. The issue you're trying to address here is in the
> > >previewer, and access to the console should be in the main UI anyway
> > >(i.e. menu entry, shortcut, and possibly toolbar action).
> Just for the KWin Scripts, we use the toolbar in order to toggle the
> konsolepreviewer. For anything else, we use the previewer in order to
> toggle the konsolepreviewer. Do you believe that we shouldn't use a
> previewer in order to toggle the konsolepreviewer?

Correct.

> > On Aug. 23, 2012, 10:19 p.m., Sebastian Kügler wrote:
> > > konsole/konsolepreviewer.cpp, line 135
> > > <http://git.reviewboard.kde.org/r/106118/diff/1/?file=79944#file79944lin
> > > e135>> >
> > >     Would be useful if the user was told what to do about it (i.e.
> > >either give the name of the expected file, or explain where you can set
> > >the name (and what it should be).
> As it regard the ui, is a KMessageBox a solution or we should put it in the
> konsole's output?

KMessageBox is fine.

> > On Aug. 23, 2012, 10:19 p.m., Sebastian Kügler wrote:
> > > mainwindow.cpp, line 870
> > > <http://git.reviewboard.kde.org/r/106118/diff/1/?file=79947#file79947lin
> > > e870>> >
> > >     Seems like a toggle button (checkable) is needed here, instead of
> > >trying to come up with whacky new concepts of doing the exact same =).
> For the KWin Scripts, we don't a previewer. So the real question is, should
> we hide previewer action or disabled(like a grey action which isn't
> clickable.)

I think hiding it is OK here.
-- 
sebas

http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9