> 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? > > Albert Astals Cid wrote: > Settings -> Configure -> General -> Program features looks good to me. > Waht you think? > > Jonathan Doman wrote: > That configure dialog is for the okular part, while the setting is only > applicable to the shell (and not other part users like konqueror). Seems like > it should just be a checkbox in the Settings menu since there are no other > shell options. > > Albert Astals Cid wrote: > You're right, but on the other hand the shell/part split is something > technical, the user should not care much. > You're also right in which we should not show the option in other shells > like konqueror. > > Still I don't think a settings checkbox menu is the best option. My idea > of the ideal scenario would be the part providing an interface as the ones in > interfaces/ where you can add your own configuration stuff to the dialog. > > But for the sake of not having this review last forever, ok, let's do the > checkbox in the settings menu and I'll see if I can implement my idea.
I agree with everything you said. For now, I have added a checkbox next to the FullScreen setting. It opens documents in a new tab by default and can be unchecked to use new windows instead. > 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. > > Albert Astals Cid wrote: > Personally i don't feel we need that warning, but ok. Comment was removed when refactoring > 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? > > Albert Astals Cid wrote: > 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. Okular::Part now emits a signal after calculating the mimetype in openFile(). This is used by the Shell to set the tab icon. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/110914/#review46407 ----------------------------------------------------------- On Jan. 9, 2014, 12:15 a.m., Jonathan Doman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/110914/ > ----------------------------------------------------------- > > (Updated Jan. 9, 2014, 12:15 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 > ----- > > part.h 4b3aafdb637080ae81eb0e45742f53a34738984d > part.cpp 05a0a62265c7e7ed79d719a3f648850f8ef642e5 > shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 > shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 > shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 > > 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
