> On Jan. 22, 2012, 7:12 p.m., Albert Astals Cid wrote: > > Hi, me again, sorry for the delay. > > > > I find the current implementation a bit confusing user-wise since i can > > have the "Page number" toolbar shown and no "Page number" toolbar is really > > visible at all since the other page widget is visible. > > For that i'm suggesting two solutions: > > a) Do not force the limit of only one minibar available, so the user can > > have none, one or two page widgets visible > > b) Uncheck the "Page number" toolbar action when the "old" page widget is > > visible, and then if somebody checks it, toggle it back > > > > The option b) looks difficult-ish since you'd probably have to do quite a > > bit of xml-gui vodoo to make it work nicely, so i'm leaning towards a) > > > > What do you think? > > Jonathan Marten wrote: > No objection to doing (a), since that would avoid all of the user > confusion with only one of the tool and page bar being able to be shown at > any time. It will slightly complicate the MiniBar implementation, the > intended observerId() will have to be specified to its constructor instead of > being hardwired in minibar.h. With that being available, two MiniBar's can > be created for the page bar and for the tool each with a unique ID. If this > is acceptable in terms of code complexity and BC then I will work on the > changes. > > > Albert Astals Cid wrote: > Why do you need two different observed id? After all even if it's two > "different" widgets they are basically "mirrors" one of the other, i think it > should be doable with just one observer id. > > Jonathan Marten wrote: > Because of Document::addObserver(DocumentObserver *pObserver), which > stores the pObserver in a QMap indexed by the observerId. So adding a second > observer with the same observerId as one already existing will overwrite the > first. > It could do an insertMulti() into the map, but then many more changes > would be needed in Document to handle this. > > Albert Astals Cid wrote: > Or you could decouple the logic of the minibar from the widget itself, > sharing the same logic for both of the widgets, that would make sense imho, > but it's probably more work than you were expecting, what do you think? i'm > not totally against adding a new observer id but i'd like to avoid it unless > totally necessary > > Jonathan Marten wrote: > Do you mean having a single observer, which then broadcasts updates to as > many page number display widgets as required? That would work, but would be > even more complicated - new classes and extra signal/slot connections. Seems > like a lot of extra complexity to add what appeared at first to be a simple > change. > > If having a new observer ID is acceptable, then I'll work on a patch to > do that. >
Ok, i really prefer the since Observer ID, but that's probably asking for too much, so you do the work with a new observer ID and i'll hack the rest, deal? - Albert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103427/#review10005 ----------------------------------------------------------- On Dec. 28, 2011, 8:21 p.m., Jonathan Marten wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/103427/ > ----------------------------------------------------------- > > (Updated Dec. 28, 2011, 8:21 p.m.) > > > Review request for Okular. > > > Description > ------- > > The bug (and duplicates) suggests that the page bar which normally appears at > the bottom of the Okular window could be docked in the toolbar, in order to > save vertical screen space which is especially useful on a wide screen. This > patch implements that. > > There can only be one MiniBar in existence, because of the fixed observer ID > (which must be unique). So the page tool only appears in the toolbar if the > page bar is hidden; if it is shown, then the page number and size label > appears there as before. The MiniBar is reparented in > Part::slotShowBottomBar() when the page bar is shown or hidden. > > The page tool is placed in its own toolbar by default, so that it can be > positioned or floated in accordance with the user's preference. > > There are GUI changes and a new I18N string, so this change (if accepted) > would be targeted at KDE SC 4.8.1 or later. > > > This addresses bug 279128. > http://bugs.kde.org/show_bug.cgi?id=279128 > > > Diffs > ----- > > part.rc 33d3829 > part.cpp b3b4234 > part-viewermode.rc dbd8e42 > part.h cae5af0 > > Diff: http://git.reviewboard.kde.org/r/103427/diff/diff > > > Testing > ------- > > Built Okular from master with these changes. Checked operation in both > embedded and standalone modes with page bar shown or not, with a variety of > PDF files. > > > Screenshots > ----------- > > With page bar shown > http://git.reviewboard.kde.org/r/103427/s/372/ > With page bar hidden > http://git.reviewboard.kde.org/r/103427/s/373/ > > > Thanks, > > Jonathan Marten > >
_______________________________________________ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
