> On Dec. 29, 2013, 7:44 p.m., Albert Astals Cid wrote:
> > shell/shell.cpp, line 203
> > <https://git.reviewboard.kde.org/r/110914/diff/6/?file=225169#file225169line203>
> >
> >     I'd like if you kept the OpenInNewTab/OpenInNewShell in a configuration 
> > setting so people were not forced to use tabs if they don't like them
> 
> Jonathan Doman wrote:
>     Where should the interface for this setting be?

Settings -> Configure -> General -> Program features looks good to me. Waht you 
think?


> On Dec. 29, 2013, 7:44 p.m., Albert Astals Cid wrote:
> > shell/shell.cpp, line 589
> > <https://git.reviewboard.kde.org/r/110914/diff/6/?file=225169#file225169line589>
> >
> >     This doesn't seem the best thing, given that okularpart knows the 
> > mimetype once it opens the file, i'd prefer a signal beign emmited instead 
> > another KMimeType call.
> 
> Jonathan Doman wrote:
>     Is KMimeType expensive? Are you suggesting that I add a new signal to 
> Okular::Part?

It is not expensive, but we're doing quite a few "tricky" things in the 
mimetype determination side besides, and that would probably like when you open 
stuff form the stdin piping, so not doing the same twice seems like a good 
idea. Yes, i'm suggesting a new signal or add a paramater to an existing one if 
you find something suitable.


> On Dec. 29, 2013, 7:44 p.m., Albert Astals Cid wrote:
> > shell/shell.cpp, line 551
> > <https://git.reviewboard.kde.org/r/110914/diff/6/?file=225169#file225169line551>
> >
> >     I don't understand the comment
> 
> Jonathan Doman wrote:
>     Using sender() is generally considered a Bad Practice, but it's simpler 
> than using a signal mapper (greatly so if we will ever allow the tabs to be 
> moved/reordered). The comment is there for others who may be inclined to 
> copy/paste the code.

Personally i don't feel we need that warning, but ok.


- Albert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/110914/#review46407
-----------------------------------------------------------


On Jan. 3, 2014, 2:24 a.m., Jonathan Doman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/110914/
> -----------------------------------------------------------
> 
> (Updated Jan. 3, 2014, 2:24 a.m.)
> 
> 
> Review request for Okular.
> 
> 
> Bugs: 155515
>     http://bugs.kde.org/show_bug.cgi?id=155515
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> This patch adds support for a tabbed interface (multiple documents in one 
> window). The core work just adds a tab bar that switches between multiple 
> embedded okularparts, but there are many other considerations:
>  - Tab context menu allows for duplicating or detaching (detached tabs start 
> in new okular process)
>  - `okular file.pdf` will open file in existing window if possible, unless 
> --new flag is used. It also selects the most recently raised/activated window 
> to use. This mirrors behavior I expect from browsers and other tabbed 
> interfaces.
>  - Warns when closing window with multiple tabs
>  - No warning is given when opening an already open file. This is the 
> behavior I strongly prefer (and observe in other programs), but will change 
> if there is consensus otherwise.
> 
> When selecting different tools in one part, the tool selection propagates to 
> all parts, but the GUI does not reflect that. This bug is present in other 
> programs (e.g. multiple okularparts in Konqueror), so I made no attempt to 
> diagnose or fix.
> 
> One menu item was added for the multiple tab warning option. When testing 
> this, I noticed that items in the Settings menu seem to move around when 
> switching tabs, and I cannot diagnose or fix this. It seems to be related to 
> XMLGUI bug #64754. 
> 
> My development branch is also hosted at 
> https://github.com/jrmrjnck/okular-tabbed
> 
> 
> Diffs
> -----
> 
>   shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 
>   shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 
> 
> Diff: https://git.reviewboard.kde.org/r/110914/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jonathan Doman
> 
>

_______________________________________________
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Reply via email to