Re: [Okular-devel] Review Request 127366: Resize annotations

2016-03-13 Thread 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 18). 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
> 
>

___
Ok

Re: [Okular-devel] Review Request 127351: Added ability to view embedded files

2016-03-13 Thread Albert Astals Cid

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



Could you please fix all the indentation to be like the one in the file?

- Albert Astals Cid


On March 12, 2016, 8:52 a.m., Daniel Lichtenberger wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127351/
> ---
> 
> (Updated March 12, 2016, 8:52 a.m.)
> 
> 
> Review request for Okular.
> 
> 
> Bugs: 295567
> http://bugs.kde.org/show_bug.cgi?id=295567
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> The selected embedded file(s) are extracted to QDir::tempPath() and the
> default application associated with the file type is launched with KRun.
> 
> Double-click on an embedded file also launches the view action.
> 
> The temporary files are cleaned up automatically when the dialog is
> closed.
> 
> 
> Diffs
> -
> 
>   ui/embeddedfilesdialog.h f3e2d038f4ffe2b64af15c94c7ec0cfb8a68834c 
>   ui/embeddedfilesdialog.cpp b400e879b2849a931e069669c984a6b578ffd536 
>   ui/guiutils.h fffe04488b27d92d9a7bc623da653d0c1df87ae8 
>   ui/guiutils.cpp 3bf3ad64b57396792c3aa33298265561ee35ed1a 
> 
> Diff: https://git.reviewboard.kde.org/r/127351/diff/
> 
> 
> Testing
> ---
> 
> Tested with 
> 
> http://www.opf-labs.org/format-corpus/pdfCabinetOfHorrors/fileAttachment.pdf 
> 
> and the PDF file in
> 
> http://www.sdbtransfer.de/app/download/10536876499/SuperSauber1_EDASXbau.zip?t=1453751464
> 
> 
> Thanks,
> 
> Daniel Lichtenberger
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel


Re: [Okular-devel] Review Request 127328: Fix build on Windows (MinGW)

2016-03-13 Thread Thomas Friedrichsmeier

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

(Updated March 14, 2016, 12:17 a.m.)


Review request for Okular, Aleix Pol Gonzalez, Alex Richardson, and Gleb Popov.


Changes
---

Albert: Adding Gleb since he did the okularpart_EXPORTS thing in mobile/ and 
apol/arichardson since afaik they're the ones that did the porting for the 
plugins/lib/part loading.


Repository: okular


Description
---

Three parts to this patch:
1) The plaform #ifdef in interfaces/viewerinterface.h is platform, not 
compiler-specific.
2) When compiling okularplugin, the existing definition of okularpart_EXPORTS 
on the generated settings.cpp does not work with MinGW, for reasons that I do 
not understand. Setting the definition on the whole target, instead, works.
3) The okularpart lib will automatically be named libokularpart, and 
subsequently not be found when trying to load it via KPluginLoader. Tell cmake 
to drop the "lib"-prefix.

See also:
- Discussion on list 
https://mail.kde.org/pipermail/okular-devel/2016-March/022480.html
- A previous review request addressed issue 2 for MSVC 
https://git.reviewboard.kde.org/r/125742/


Diffs
-

  CMakeLists.txt e17bc25 
  interfaces/viewerinterface.h e9e76a2 
  mobile/components/CMakeLists.txt e09326a 

Diff: https://git.reviewboard.kde.org/r/127328/diff/


Testing
---

Builds on Windows with MinGW. Starts, and loads okularpart, successfully.


Thanks,

Thomas Friedrichsmeier

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel


Re: [Okular-devel] Review Request 127013: Properly show marker for current section in TOC

2016-03-13 Thread Albert Astals Cid


> On March 10, 2016, 12:28 a.m., Albert Astals Cid wrote:
> > This will put triangles in the whole chain of parent, child, child when the 
> > list is not a treeview, maybe we can also check somewhere else so that the 
> > behaviour does not change when it's not a treeview (i.e. onyl the last one 
> > has the triangle)?
> 
> Miklós Máté wrote:
> Without information about the expanded nodes the best we can do is to put 
> triangles at every containing node. Using only the last node is not good, 
> because it may be hidden within a collapsed parent node.

I disagree, please change it so that it behaves as it used to behave if it's 
not a qtreeview.


- Albert


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


On March 9, 2016, 8:41 p.m., Miklós Máté wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127013/
> ---
> 
> (Updated March 9, 2016, 8:41 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> fixes bug #342076
> 
> 
> Diffs
> -
> 
>   ui/tocmodel.cpp ce93366 
> 
> Diff: https://git.reviewboard.kde.org/r/127013/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Miklós Máté
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel


[Okular-devel] Review Request 127366: Resize annotations

2016-03-13 Thread Tobias Deiminger

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

Review request for Okular.


Repository: okular


Description
---

This diff adds a first version of an annotation resize feature to okular (see 
Bug 18). 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