<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127366/">https://git.reviewboard.kde.org/r/127366/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 14th, 2016, 12:46 a.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Looks very nice for a start, congratulations.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Reconsider UI, esp. the Ctrl-way to get focus. Is there something in the KDE HIG about it?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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)</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Improve handling when annotation gets so small that resize handles overlap.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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?</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Find approach how to handle "resize to negative geometry".</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think not letting people go negative is what makes more sense</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Known Bug: In the thumbnail list, resize handles are drawn too big, and not refreshed correctly.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'd say it makes more sense to not draw the handles at all</p></pre>
</blockquote>
<p>On March 18th, 2016, 8:33 p.m. UTC, <b>Tobias Deiminger</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Changed:
-resize handles are filled slightly transparent</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Btw., sorry, I don't know how inline quote here (using the reviewboard webinterface).</p></pre>
</blockquote>
<p>On March 21st, 2016, 7:20 a.m. UTC, <b>Jonathan Schultz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On March 21st, 2016, 11:03 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It's for the "mobile" version of the app, it doesn't have the complex interaction modes (i think) so ignore for now.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Btw., sorry, I don't know how inline quote here (using the reviewboard webinterface).</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It's using markdown, see the "Markdown Reference" link, but it's basically prefixing the line by > and then having an empty line for adding your answer</p></pre>
</blockquote>
<p>On March 26th, 2016, 8:25 p.m. UTC, <b>Tobias Deiminger</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">annotations should be select-able, by left-clicking on some obvious part of them, and once selected can then be moved/resized</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Calligra and Karbon do it like Jonathan says. I liked the idea too, so gave it a try and find the handling better now. What do you think?
Could there be problems because of conflicting user intents (e.g., move the whole document vs. move the annotation)?
I can quite easily revert or change it again, as design has also improved now.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">it overlaps with stuff I am working on to do more with selections</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sounds interesting... who would eventually care about merging?</p></pre>
</blockquote>
<p>On April 28th, 2016, 7:48 p.m. UTC, <b>Tobias Deiminger</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Albert, can you already vote for or against following changes? (concerns rev 5 of the diff)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">1) The patch changes the UI for annotations: Away from "hold ctrl-key while operating on", towards "select by left click, then operate on". See "Usage" in descirption of this RR, or just apply the patch and try it. It's similiar to other KDE applications (calligra flow, karbon, words, ...), and I find it more self-explaining. On the other hand, it breaks behavior okular users might be used to. If in doubt, rev 5 should be suitable for giving a working prototype to kde-usability list and ask them.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">2) The patch considerably modifies class PageView. The idea is to have all "operate on annotation" functionality in class MouseAnnotation, instead of interleaving it in the longish PageView. PageView then only delegates events to MouseAnnotation. This concerns also non-resize-related features. E.g. I moved annotation tooltips and video playback there. It was almost necessary for me to gain an overview of existing features and behavior, and it should aid keeping consistency and avoiding conflicts in the UI. It also helps to identify and separate common code. On the other hand there's more to review for you, and there's higher risk of introducing regressions.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Are both changes OK for you? If so, I'd go on with identifiying missing things, tidying up and testing.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If you don't agree, I'll stop for now and wait until there's time for discussion on alternate approaches.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Cheers
Tobias</p></pre>
</blockquote>
<p>On April 28th, 2016, 11:18 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sorry, i have a huge backlog of thigns to do, will try to get this review done as soon as possible :/</p></pre>
</blockquote>
<p>On October 9th, 2016, 11:31 p.m. UTC, <b>Carl-Daniel Hailfinger</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Tested against latest git, works fine. The "select by left click, then operate on" UI is pretty nice.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You may have to regenerate the patch. Some files started to show offsets when applying.</p></pre>
</blockquote>
<p>On October 25th, 2016, 7:58 a.m. UTC, <b>Oliver Sander</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">May I politely ask about the status of this? Does it still need further review, and manpower is lacking?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The patch applies to the master branch. Will it get merged first, and then forward-ported to the frameworks branch?
Or does it need to get ported to KF5 first, and then get merged directly into frameworks (skipping master)?</p></pre>
</blockquote>
<p>On October 25th, 2016, 9:19 p.m. UTC, <b>Tobias Deiminger</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is up to the okular developer team, so I just join Olivers polite ping here. From my side there are some minor changes and bug fixes pending. Some more effort should go into considering and testing impact on more exotic annotation subtypes. I'll stay tuned and will try to help once there's time for further actions.</p></pre>
</blockquote>
<p>On October 28th, 2016, 12:04 a.m. UTC, <b>Jonathan Schultz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm also pretty interested in this issue as part of a broader agenda of building a more elaborate coding system based on Okular. I won't go into more detail here, except to say that I think a lot of useful stuff like Tobias' patch could usefully be incorporated into Okular. So FWIW I'm happy to do some building/testing/reviewing/extending as may help to make progress.</p></pre>
</blockquote>
<p>On November 10th, 2016, 8:01 a.m. UTC, <b>Oliver Sander</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The patch does not apply anymore now that the frameworks branch has been merged into master. Would it be able for you rebase the patch and do the necessary changes to make it work with QT5 etc? Thanks!</p></pre>
</blockquote>
<p>On November 14th, 2016, 8:57 a.m. UTC, <b>Oliver Sander</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Just tested the new frameworks version, and it seems to work very nicely.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Can we please get code review and have this merged? Is there anything I can do to help?</p></pre>
</blockquote>
<p>On January 23rd, 2017, 8:49 a.m. UTC, <b>Oliver Sander</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Polite ping?</p></pre>
</blockquote>
<p>On January 23rd, 2017, 10:30 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There's things marked as "Known Bugs", are those going to be fixed?</p></pre>
</blockquote>
<p>On January 24th, 2017, 8:21 a.m. UTC, <b>Tobias Deiminger</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Yes, I will fix them (or disable buggy functions if it'd turned out that fixing is too tough).
It'd be great to get green lights for the overall approach before spending more time - Albert, have you or someone from the okular team already had a chance to look at the patch in more depth?</p></pre>
</blockquote>
<p>On January 24th, 2017, 10:23 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Albert, have you or someone from the okular team already had a chance to look at the patch in more depth?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There's some obvious but easy to fix mistakes like renaming an enum and leaving the @since untouched; the variables of adjust() missing const; the virtuals in the AdjustAnnotationCommand have to be overrides, the connects using old style instead of new, etc. but this is just small stuff.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Then as a user, it's obvious that once you let me select an annotation by mouse click, i want the Delete key to delete it.</p></pre>
</blockquote>
<p>On January 26th, 2017, 9:48 p.m. UTC, <b>Tobias Deiminger</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks for the review! Rev 7 fixes the mistakes you mentioned and implements the Delete key feature. A few comments:</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">virtuals in the AdjustAnnotationCommand have to be overrides</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">They're overrides, aren't they? I double checked the signature of the virtuals from QUndoCommand and AdjustAnnotationCommand, but couldn't see a difference.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">renaming an enum and leaving the @since untouched</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">While fixing this I realized that annotation.h is part of a public API (there's even a Debian packages for it, https://packages.debian.org/sid/libokularcore7).
Renaming BeingMoved to BeingModified would break existing client code, so I'm keeping BeingMoved and add BeingResized now.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'll address the "Known bug" things soon. If you find other issues just let me know.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">They're overrides, aren't they? I double checked the signature of the virtuals from QUndoCommand and AdjustAnnotationCommand, but couldn't see a difference.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Yes, they are overrides, and thus should be marked as such, i.e. using the override keyword instead of the virtual one (since we use C++11 now)</p></pre>
<br />
<p>- Albert</p>
<br />
<p>On January 26th, 2017, 9:47 p.m. UTC, Tobias Deiminger wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Okular.</div>
<div>By Tobias Deiminger.</div>
<p style="color: grey;"><i>Updated Jan. 26, 2017, 9:47 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=177778">177778</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
okular
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This diff adds an annotation resize feature to okular (see Bug 177778).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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
Some functionality unrelated to the resize feature is also shifted to class MouseAnnotation, to establish a single place of responsibility for annotation interactions.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Known Bugs:
-A comment annotation (subtype 1) with embedded appearance can be selected for resize. But the resize changes only the selection rectangle, while the annotation is rendered unchanged.
-AWidget (subtype 13) can be selected for move and resize, but nothing happens when performing those actions.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">TODO:
-Consider z-Order for overlapping annotations? (currently, selection rectangle is always drawn on top, while the related annotation may be in background).
-Provide a PDF document with suitable content, where supported annotation types and and possible regressions can be tested.
-Add test cases once requirements are fixed.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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°)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Selection is canceled
-when currently selected annotation is deleted
-on mouse click outside of currently selected annotation
-ESC is pressed</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Viewport is shifted when mouse cursor during move/resize comes close to viewport border.
Resize to negative is prevented.
Tiny annotations are still selectable.
If mouse is moved over an annotation type that we can focus, and the annotation is not yet focused, mouse cursor shape changes to arrow.
If mouse cursor rests over an annotation A, while annotation B is focused, a tooltip for annotation A is shown.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Test for regressions:
-Annotation interaction (focus, move, resize, start playback, ...) are only done in mode EnumMouseMode::Browse.
-If mouse is moved over an annotation type where we can start an action, mouse cursor shape changes to pointing hand.
-If mouse is moved over an annotation type that we can't interact with, mouse cursor shape stays a open hand.
-If mouse cursor rests over an annotation of any type, a tooltip for that annotation is shown.
-Grab/move scroll area (on left click + mouse move) is prevented, if mouse is over focused annotation, or over AMovie/AScreen/AFileAttachment annotation.
-A double click on a annotation starts the "annotator".</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(78a2e855c22ebb31a9195c679cf0adf981d5443f)</span></li>
<li>core/annotations.h <span style="color: grey">(5653097fe892ce57c8e81615a1c20217e538e1de)</span></li>
<li>core/annotations.cpp <span style="color: grey">(c95fe877316398a5341e29146379dd231dfa40c7)</span></li>
<li>core/annotations_p.h <span style="color: grey">(07b124a4fae40b7a983aa382ae824125e6d25746)</span></li>
<li>core/document.h <span style="color: grey">(bac38f89f85980d478e6252ecc8dc823cbe4359a)</span></li>
<li>core/document.cpp <span style="color: grey">(468a8a9fbadaef5ca8540cf113d5397b989f2aa5)</span></li>
<li>core/document_p.h <span style="color: grey">(8b20c5586f641f6f9ec3a6a6f0d978848a2cb7c8)</span></li>
<li>core/documentcommands.cpp <span style="color: grey">(aafc45a1a989a7240520f2168e253d51d6744f7c)</span></li>
<li>core/documentcommands_p.h <span style="color: grey">(616999dd68406590b304cf648878fa8acb3ec6e0)</span></li>
<li>generators/poppler/annots.cpp <span style="color: grey">(df67986adc076f722e601c3bea187200ecf9df31)</span></li>
<li>ui/pagepainter.cpp <span style="color: grey">(c5eb1ef90e4af48c644a7890a60fa3892cdcf08a)</span></li>
<li>ui/pageview.cpp <span style="color: grey">(8992539c7c282e865a3e5d20e119c8790d79e925)</span></li>
<li>ui/pageviewmouseannotation.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/pageviewmouseannotation.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127366/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>