D21734: [WIP] Improve class documentation for PageView and PageViewItem

[David Hurka]

davidhurka added a comment.


  Documenting these classes wasn’t really difficult, they already have helpful inline comments and member names.
  
  I think PageViewItem should store page rotation, along with already stored geometry information, which is rotation dependent.
  That will make it easier to implement //both// Bug 307224 (Ability to rotate single pages) and 169847 (split view feature, second most wished feature request :) ).
  Page could store the information too, but that is semantically less correct.

INLINE COMMENTS

> pageview.cpp:429
>      d->leftClickTimer.setSingleShot( true );
>      connect( &d->leftClickTimer, &QTimer::timeout, this, &PageView::slotShowSizeAllCursor );
>  

Wrong cursor? Dragging the page activates a closed hand, not size all.

> pageview.cpp:485
>      d->aZoom->setEditable( true );
>      d->aZoom->setMaxComboViewCount( 14 );
>      connect( d->aZoom, SIGNAL(triggered(QAction*)), this, SLOT(slotZoom()) );

This has usually 17 items. Any reasons to limit to 14 instead?

> pageview.cpp:3717
>      {
>          const QRect & r = i->croppedGeometry();
>          if ( x < r.right() && x > r.left() && y < r.bottom() )

Suggestion: QRect::contains(x, y, proper = true)

REPOSITORY
  R223 Okular

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

To: davidhurka
Cc: okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20190610/b1b37ddb/attachment.html>