[Okular-devel] Review Request 110566: Configurable review tools

[Albert Astals Cid]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110566/#review32856
-----------------------------------------------------------


Looks good overall, it's good taht it doesn't touch core/ at all so if we need to change something taht was horribly wrong and we didn't realize we can still do it in a minor revision. OTOH I'm kind of thinking that some of this code might make more sense in core/ (like the code that "creates" the definitions of the tools, etc) but we are always on time to move it "down" later if we need.

Some minor stuff coming as comments


conf/okular.kcfg
<http://git.reviewboard.kde.org/r/110566/#comment24382>

    Maybe add a kWarning here if annotationTools content is empty? The old code seemed to have them, no?



conf/preferencesdialog.cpp
<http://git.reviewboard.kde.org/r/110566/#comment24383>

    Instead of draw-freehand maybe use the same icon we use in the side panel?



conf/widgetannottools.cpp
<http://git.reviewboard.kde.org/r/110566/#comment24386>

    Would it make sense to try to reuse the defaultToolName function?



conf/widgetannottools.cpp
<http://git.reviewboard.kde.org/r/110566/#comment24384>

    maybe i18nc here so it is clear that this is a noun and not a verb?



ui/pageviewannotator.cpp
<http://git.reviewboard.kde.org/r/110566/#comment24385>

    Maybe some i18nc here too?


- Albert Astals Cid


On May 20, 2013, 7 p.m., Fabio D'Urso wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110566/
> -----------------------------------------------------------
> 
> (Updated May 20, 2013, 7 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> -------
> 
> Diff dump from the configurable-review-tools branch, as requested in http://mail.kde.org/pipermail/okular-devel/2013-May/015009.html
> 
> This patch mainly addresses bug 159601, but it also adds GUI control to configure some annotation properties (text alignment in inline notes, stroke width in freehand lines, background color in polygons) and changes some texts.
> 
> Use
>  gitk origin/configurable-review-tools ^origin/master --no-merges
> for a detailed changelog.
> 
> 
> This addresses bug 159601.
>     http://bugs.kde.org/show_bug.cgi?id=159601
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 64c4c2a 
>   Messages.sh 6d0d0b0 
>   conf/dlgannotations.h PRE-CREATION 
>   conf/dlgannotations.cpp PRE-CREATION 
>   conf/dlgannotationsbase.ui PRE-CREATION 
>   conf/dlgidentity.h 1bbd937 
>   conf/dlgidentity.cpp 8585716 
>   conf/dlgidentitybase.ui 15752eb 
>   conf/okular.kcfg 4a2aaf3 
>   conf/preferencesdialog.h 3340487 
>   conf/preferencesdialog.cpp 9f6d339 
>   conf/settings.kcfgc 060f260 
>   conf/widgetannottools.h PRE-CREATION 
>   conf/widgetannottools.cpp PRE-CREATION 
>   ui/annotationpropertiesdialog.h d1a1c27 
>   ui/annotationpropertiesdialog.cpp 5d86d79 
>   ui/annotationwidgets.h 1832876 
>   ui/annotationwidgets.cpp ce8a91b 
>   ui/data/CMakeLists.txt 6501be5 
>   ui/data/sources/tool-base-okular.svgz PRE-CREATION 
>   ui/data/sources/tool-highlighter-okular-colorizable.svgz PRE-CREATION 
>   ui/data/sources/tool-ink-okular-colorizable.svgz PRE-CREATION 
>   ui/data/sources/tool-note-inline-okular-colorizable.svgz PRE-CREATION 
>   ui/data/sources/tool-note-okular-colorizable.svgz PRE-CREATION 
>   ui/data/tool-base-okular.png PRE-CREATION 
>   ui/data/tool-ellipse-okular.png 6a3260e 
>   ui/data/tool-highlighter-okular-colorizable.png PRE-CREATION 
>   ui/data/tool-highlighter-okular.png 594ba41 
>   ui/data/tool-ink-okular-colorizable.png PRE-CREATION 
>   ui/data/tool-ink-okular.png 8a2eeb0 
>   ui/data/tool-line-okular.png a2dda94 
>   ui/data/tool-note-inline-okular-colorizable.png PRE-CREATION 
>   ui/data/tool-note-inline-okular.png 4d8187f 
>   ui/data/tool-note-okular-colorizable.png PRE-CREATION 
>   ui/data/tool-note-okular.png a89c91b 
>   ui/data/tool-polygon-okular.png 66ba2cb 
>   ui/data/tool-stamp-okular.png e53a04a 
>   ui/data/tool-underline-okular.png 924772f 
>   ui/data/tools.xml 5e3cb84 
>   ui/guiutils.h 73c0838 
>   ui/guiutils.cpp af06000 
>   ui/pagepainter.h 23ac845 
>   ui/pagepainter.cpp 2890b56 
>   ui/pageview.cpp 26f5516 
>   ui/pageviewannotator.h 850d887 
>   ui/pageviewannotator.cpp 035c1f3 
>   ui/pageviewutils.h 0aaf057 
>   ui/pageviewutils.cpp c2e0388 
> 
> Diff: http://git.reviewboard.kde.org/r/110566/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Fabio D'Urso
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20130521/3196f735/attachment.html>