[Okular-devel] Review Request 127366: Resize annotations

[Albert Astals Cid]

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



Looks very nice for a start, congratulations.

> Reconsider UI, esp. the Ctrl-way to get focus. Is there something in the KDE HIG about it?

It's not too bad, i think it would be ok, but if you awnt to try asking in the kde-usability mailing list it may help (sometimes it does not though, so be prepared :D)


> Improve handling when annotation gets so small that resize handles overlap.

I guess in that case one could try to always default to one of the corner ones, so you're forced to make it bigger by 2 dimensions?


> Find approach how to handle "resize to negative geometry".

I think not letting people go negative is what makes more sense


> Known Bug: In the thumbnail list, resize handles are drawn too big, and not refreshed correctly.

I'd say it makes more sense to not draw the handles at all

- Albert Astals Cid


On March 13, 2016, 8:25 p.m., Tobias Deiminger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127366/
> -----------------------------------------------------------
> 
> (Updated March 13, 2016, 8:25 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> This diff adds a first version of an annotation resize feature to okular (see Bug 177778). Do you think the approach is worth carrying on? If so, I'm looking forward to read your comments on everything below, esp. the TODO items.
> 
> Usage:
> If you press Ctrl while mouse cursor is over an annotation, you'll get focus on that annotation. Now keep Ctrl pressed, and move mouse cursor over one of the 8 resize handles (each 10x10 pixels) on the corners/edges. The cursor will change shape to indicate resize mode. Clicking somewhere else on the annotation means "move", just as it was before resize feature was added.
> 
> Notable changes:
> It works by eventually changing AnnotationPrivate::m_boundary and notifying generator (i.e. poppler) about that change, similar to the existing move functionality.
> -Separated annotation state handling out of PageView into a new class MouseAnnotation (ui/pageviewmouseannotation.cpp).
> -Added method Document::resizePageAnnotation, backed by a QUndoCommand class Okular::ResizeAnnotationCommand.
> -Added method Annotation::resize functions and new Annotation flags BeingResized and BeingFocused.
> -Draw resize handles in PagePainter::paintCroppedPageOnPainter
> -Draw only a bounding rectangle during resize, if annotation is rendered externally
> 
> TODO:
> -Reconsider UI, esp. the Ctrl-way to get focus. Is there something in the KDE HIG about it?
> -Feature is currently limited to annotation of type AText and AStamp => extend to all types where applicable.
> -Improve handling when annotation gets so small that resize handles overlap.
> -Find approach how to handle "resize to negative geometry". Maybe flip resize mode, e.g. from top left to bottom right when hitting size 0. Or just deny further lessening at size 0.
> -Reconsider name of class MouseAnnotation. What does it do, and what's a good name for that?
> -Reconsider interface between PageView and MouseAnnotation (currently it's just forwarding UI events together with PageViewItem and event positions).
> -Add test cases once requirements are fixed.
> -Known Bug: In the thumbnail list, resize handles are drawn too big, and not refreshed correctly.
> -Known Bug: When page orientation is changed (e.g. rotated by 90 deg.), wrong resize mode is applied.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 97e8db6e4a704fd34331fad7b7628ca2248b62d8 
>   core/annotations.h 4f107440dc824fd9049a30082befd18642e63895 
>   core/annotations.cpp e02994688414bdf485b308d6ef122ee2eff3fbaf 
>   core/annotations_p.h 07b124a4fae40b7a983aa382ae824125e6d25746 
>   core/document.h 63f58741bd6680a673945a7b7c05a10130968beb 
>   core/document.cpp 6953b1fb0dc29a375be7ff331a2a2bccce975366 
>   core/document_p.h fda23275033645ea67f5ad9d27341fc4635ede34 
>   core/documentcommands.cpp 95aded51d73a3d3b98ff26284c9c46fc5c9cf0ca 
>   core/documentcommands_p.h 17394f2a25b187cf4aff66b3a7f891b81be5acdd 
>   generators/poppler/annots.cpp 8cde64833831ec833b3be552608cff99d38f8e63 
>   ui/pagepainter.h 68b241658162d9bd6eb187efc594ef17ea99d899 
>   ui/pagepainter.cpp 57e8620cdefc36e40660fce242d7ea8197c25339 
>   ui/pageview.cpp 3ebf7dcb04aa1942e02f49133d98081e2bbb565c 
>   ui/pageviewmouseannotation.h PRE-CREATION 
>   ui/pageviewmouseannotation.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/127366/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tobias Deiminger
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20160314/24367e30/attachment.html>