Review Request 126685: Add zoom support for documentation view

[Milian Wolff]

On Samstag, 23. Januar 2016 17:07:39 CET Igor Kushnir wrote:
> > On Jan. 11, 2016, 11:37 a.m., Aleix Pol Gonzalez wrote:
> > > Wouldn't it be ok just implementing it on a per-view basis using
> > > ctrl+mouse wheel?> 
> > Kevin Funk wrote:
> >     Note: We have two submissions for fixing this bug.
> >     
> >     There's also: https://phabricator.kde.org/D774 (with a much less
> >     intrusive solution, which I like better)> 
> > Igor Kushnir wrote:
> >     I added zoom-in and zoom-out buttons because of Kevin's suggestion:
> >     https://bugs.kde.org/show_bug.cgi?id=285162#c11 These buttons won't
> >     be really useful for everyone's workflow since the mouse wheel is
> >     enough. They could improve discover-ability of the feature though. If
> >     the zoom feature would have been implemented on per-view basis, I'm
> >     not sure how zoom factor could be synchronized between different
> >     views. If it won't be synchronized at all, then this zoom factor
> >     should also be stored in KConfig for each view separately. But how to
> >     determine a different KConfig entry name in each view? Pass a string
> >     as a parameter instead of the DocumentationZoom object? Even then,
> >     having to adjust all views' zoom factors separately would be
> >     inconvenient for the user. Another solution is a static/global
> >     zoomFactor variable. I implemented something similar here:
> >     https://bugsfiles.kde.org/attachment.cgi?id=85775 (this is a patch
> >     attached to the issue). But I don't really like globals, and I didn't
> >     see much of those in the kdevplatform code.
> >     
> >     Unfortunately, I can't access https://phabricator.kde.org/D774 to
> >     evaluate the alternative implementation, because this particular page
> >     seems to be protected and I don't have an account there.> 
> > Igor Kushnir wrote:
> >     I think that the alternative implementation is good too.
> >     With respect to behavior, the biggest differences are:
> >     1) the extra tool button (+1 functionality, but less space available);
> >     2) horizontal scroll is not handled (which is interpreted in Qt
> >     Assistant and firefox in the way I implemented it). 3) larger/smaller
> >     wheel scroll steps are not handled. I couldn't see these different
> >     steps during debugging, but Qt documentation suggests that some
> >     devices support more fine-grained scrolling.
> >     
> >     There are indeed less changes and they are less intrusive in Sergey's
> >     solution. But on the other hand, my implementation fits better in the
> >     existing architecture (zoom feature works similarly to the existing
> >     search feature), does not employ a qobject_cast trick and can support
> >     documentation widgets that don't use StandardDocumentationView.
> >     
> >     I'm not proficient in a proper KConfig use, so I guess my
> >     implementation is worse in this regard.
> >     
> >     We could either merge these 2 solutions by choosing the best aspects
> >     of each (as defined by reviewers). Or maybe Sergey would want to
> >     enhance the implementation in a way suggested by Kevin in his review
> >     (I don't really have much time at the moment to re-implement
> >     StandardDocumentationView).
> I made changes on top of this review request in several branches.
> In one branch, a KToolBar mActions is eliminated, simple QActions are added
> via DocumentationView::addAction; QComboBox mProviders and QLineEdit
> mIdentifiers are added in a QHBoxLayout *under* the
> back/forward/find/home/zoom icons. In another branch, a QToolBar
> addressActions is added *above* mActions; mProviders and mIdentifiers are
> moved to addressActions. See screenshots here:
> https://drive.google.com/uc?export=download&id=0B0F-aqLyFk_PaGVxWU54cWJaUTA
> Let me know which implementation looks best, and I'll update this review
> request with the corresponding branch.

Hey Igor,

I think the toolbar is too crowded, i.e. the actual line edit to enter a 
search string has not enough space. Can you improve that, e.g. by removing 
texts from the buttons? Take inspiration from how the KTextEditor "find" bar 
looks like (i.e. press ctrl + f in an editor).

I'm not too sure whether having a "case sensitive" toggable is required, just 
always doing case-insensitive searches in documentations sounds reasonable to 
me.

Bye

-- 
Milian Wolff
mail at milianw.de
http://milianw.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20160123/d02b83d3/attachment.sig>