Review Request 126685: Add zoom support for documentation view

[Igor Kushnir]

> 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).
> 
> Igor Kushnir wrote:
>     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.

Removed text from buttons on the documentation find widget. I don't like the idea to remove the case-insensitive functionality, so I haven't done this. The find widget patch is totally independent from this review request, so I will create a separate review request. This is a link to screenshots with zoom and search patches combined: https://drive.google.com/uc?export=download&id=0B0F-aqLyFk_PQU05b1ItbGo3aEU
Please choose the best-looking documentation zoom implementation, assuming that it is combined with the search patch.


- Igor


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


On Jan. 9, 2016, 6:56 p.m., Igor Kushnir wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126685/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2016, 6:56 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Add zoom support for documentation view
> 
> 
> Diffs
> -----
> 
>   documentation/CMakeLists.txt ff57e258ab5c62ce737c4013005247d585e87b61 
>   documentation/documentationview.h 5f7f6d8bb100ca3f2804cb716a6a4fe98c8ad05e 
>   documentation/documentationview.cpp 9d184a071a49f25ce28f877ffb16c3d2c0b9f1f5 
>   documentation/documentationzoom.h PRE-CREATION 
>   documentation/documentationzoom.cpp PRE-CREATION 
>   documentation/standarddocumentationview.h ba454427165a0df8372fa6d51ebc714423845107 
>   documentation/standarddocumentationview.cpp a9185fd592771f9ead7d4216e08f8c93abab601a 
>   interfaces/idocumentation.h 7673e5fe58ab736d864328b0c8c6b087b197867b 
> 
> Diff: https://git.reviewboard.kde.org/r/126685/diff/
> 
> 
> Testing
> -------
> 
> Built, installed, run tests.
> 
> 
> Thanks,
> 
> Igor Kushnir
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20160123/10f9fdbf/attachment-0001.html>