[Okular-devel] Review Request 127366: Resize annotations

Fri Apr 8 06:17:11 UTC 2016

> On April 7, 2016, 9:52 nachm., Jonathan Schultz wrote:
> > Sorry I've been meaning to test this a bit more and only got to it today.
> > 
> > I have a couple of minor comments:
> > 
> > 1. The corner boxes do not seem to work - even though the mouse icon changes to indicate resizing, left-clicking causes the annotation to become unselected. That looks like such an obvious thing that perhaps it is a bug that has crept in, or is strangely dependent on my particular build?
> > 
> > 2. The mouse icon changes could be made much better. Could the cursor not first (ie before an annotation has been selected) change to a pointer when over a point that left-clicking would select an annotation? When an annotation is selected, the "move annotation" (ie the vertical cross) is currently maintained even when the mouse cursor is no longer over the annotation itself, ie that left-clicking will not produce movement of the annotation.
> > 
> > More generally in terms of UI I really like the "select then operate on" model, and would suggest that, once an annotation has been selected, a range of functions (most obviously delete/edit/properties of annotation but also cut/copy and others) then become accessible through a context-dependent right-click menu?
> > 
> > > Sounds interesting... who would eventually care about merging?
> > 
> > I think you are way ahead of me, and I am looking forward to going over your patch to learn how to do a few things. That said, I have contributed one tiny review request: https://git.reviewboard.kde.org/r/127496/ but haven't had any response yet.

Thanks a lot for testing!

> left-clicking causes the annotation to become unselected

I can't reproduce it yet. The last diff is a few commits behind now. But even after rebase and clean build it correctly starts resizing after left click for me.

> Could the cursor not first (ie before an annotation has been selected) change to a pointer when over a point that left-clicking would select an annotation?

Yes, I'd like that too, will try it. It means to check for what's under the mouse more frequently (with every move event, not just every click event), but that's not too bad I guess. Finally it'll be up to Albert and his team whether they like the UI.

> (ie the vertical cross) is currently maintained even when the mouse cursor is no longer over the annotation

Again, I could not reproduce that. There seems to be some difference between your build and mine. I'll try to merge the diff into a clean clone and see what happens then.

> I am looking forward to going over your patch to learn how to do a few things.

I'm also at learning, don't trust too much in my changes ;-)

> https://git.reviewboard.kde.org/r/127496/ but haven't had any response yet.

Can't promise when, but will try to test yours too, thanks again!

- Tobias

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

On April 5, 2016, 8:02 vorm., Tobias Deiminger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127366/
> -----------------------------------------------------------
> 
> (Updated April 5, 2016, 8:02 vorm.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> This diff adds an annotation resize feature to okular (see Bug 177778).
> 
> Usage:
> If you left-click at an annotation, it gets selected and 8 resize handles appear on the corners/edges of the selection rectangle. When cursor is moved over one of the handles, the cursor shape indicates resize mode (everywhere else on the annotation means "move", just as it was before resize feature was added). Press ESC, or click an area outside the annotation to cancel selection. Feature is only applicable for annotation types AText, AStamp and AGeom.
> 
> 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::adjustPageAnnotation, backed by a QUndoCommand class Okular::AdjustAnnotationCommand
> -Added method Annotation::adjust
> -Draw resize handles and selection boundary in MouseAnnotation::routePaint
> -Draw only a bounding rectangle during resize, if annotation is rendered externally
> 
> TODO:
> -Add test cases once requirements are fixed.
> 
> 
> 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 
>   tests/translateannotationtest.cpp 184b9474e6072a991a5ee5f1116bf7a9ef10cadc 
>   ui/pagepainter.h 68b241658162d9bd6eb187efc594ef17ea99d899 
>   ui/pagepainter.cpp 3bcd8bc4cfe7471bc3c21cfcd3cff50b8a8d49ee 
>   ui/pageview.cpp 3ebf7dcb04aa1942e02f49133d98081e2bbb565c 
>   ui/pageviewmouseannotation.h PRE-CREATION 
>   ui/pageviewmouseannotation.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/127366/diff/
> 
> 
> Testing
> -------
> 
> Resize and move work
> -for types AText, AStamp and AGeom
> -on all pages of document
> -when viewport position changes
> -when zoom level changes
> -for all page rotations (0°, 90°, 180°, 270°)
> 
> Selection is canceled
> -when currently selected annotation is deleted
> -on mouse click outside of currently selected annotation
> -ESC is pressed
> 
> Viewport is shifted when mouse cursor during move/resize comes close to viewport border.
> Resize to negative is prevented.
> Tiny annotations are still selectable.
> 
> 
> Thanks,
> 
> Tobias Deiminger
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20160408/513248e1/attachment.html>