> On March 14, 2016, 12:46 a.m., Albert Astals Cid wrote: > > 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 > > Tobias Deiminger wrote: > Fixed with Revision 2: > -set correct resize mode if page is rotated > -don't allow resize to smaller than 0 x 0 > -don't draw resize handles in thumbnail list > > Changed: > -resize handles are filled slightly transparent > > Drawing the handles is now optional, and only enabled in > PageView::drawDocumentOnPainter. > Should it be done somewhere else, too? E.g., there is > active/components/pageitem.cpp, but sadly I don't have a clue when/by whom > this is used. > > Regarding kde-usability mailing list, I think before posting there I'll > try to find some good KDE reference application and check what they do. I'd > consider Okular as a good reference app, but it won't help me in this case > :-) Some ideas come already from gwenview. > > Is there something that you think should be done soon, or as next change? > If not I'll just go on with what comes to my mind, and will probably update > here in 1..2 weeks. > > Btw., sorry, I don't know how inline quote here (using the reviewboard > webinterface).
I like it a lot in terms what it lets the user do, but agree that the use of the Control key is a bit unusual. I also have an interest in this because it overlaps with stuff I am working on to do more with selections. My sense is that annotations should be select-able, by left-clicking on some obvious part of them, and once selected can then be moved/resized, (and eventually also cut/copied/deleted/properties/etc). This seems more consistent with UIs that people are used to. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127366/#review93501 ----------------------------------------------------------- On March 18, 2016, 8:06 p.m., Tobias Deiminger wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127366/ > ----------------------------------------------------------- > > (Updated March 18, 2016, 8:06 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. > -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. > > > 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 > ------- > > > File Attachments > ---------------- > > annotationresize.diff > > https://git.reviewboard.kde.org/media/uploaded/files/2016/03/18/c9141599-8eb1-4054-8eb3-d81eec44fb94__annotationresize.diff > annotationresize.diff > > https://git.reviewboard.kde.org/media/uploaded/files/2016/03/18/9d63276d-4765-406f-ad1d-6d4176b88c53__annotationresize.diff > > > Thanks, > > Tobias Deiminger > >
_______________________________________________ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
