----------------------------------------------------------- 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 > >
_______________________________________________ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
