D15775: Make the item background color and page cache properties available from View component

[Dan Leinir Turthra Jensen]

leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


  Looks pretty good to me. It'd be nice if we could avoid exposing DocumentImpl (it's supposed to be sort of hidden), if you could have a think on a way to avoid that, i'd appreciate it :)

INLINE COMMENTS

> View.cpp:139
> +    if (pageCache() == withCache)
> +        return;
> +

i'm always very uncomfortable with introducing returns inside statements... i'd very much like if you could refactor this so it doesn't just return like this (perhaps merge the two if statements, something like `if (pageCache() != withCache && d->document && d->document->impl())`?). i know it's semantically identical, but maintainability of early returns is just... super awkward ;)

> KWCanvasBase.h:106
>      virtual void setCacheEnabled(bool enabled, int cacheSize = 50, qreal maxZoom = 2.0);
> +    bool isCacheEnabled() const;
>  

It'd be nice to have docs on new public APIs, please :) i realise it's trivial and "just right there", but the api docs page doesn't show stuff quite so conveniently without a bit of help ;)

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D15775

To: dcaliste, leinir, danders, anthonyfieroni, #calligra:_3.0
Cc: boemann, Calligra-Devel-list, dcaliste, cochise, vandenoever
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20180928/f629b150/attachment.htm>