D15580: New annotation toolbar

[David Hurka]

davidhurka added a comment.


  In D15580#474457 <https://phabricator.kde.org/D15580#474457>, @simgunz wrote:
  
  > In D15580#474436 <https://phabricator.kde.org/D15580#474436>, @simgunz wrote:
  >
  > > > Or: Why is this still PageViewToolBar? It is not anymore in the PageView?
  > >
  > > I'll move it, added to TODO
  >
  >
  > Actually the code of the other toolbar (Browse, Zoom, Selection) is managed in the file pageview.cpp. In which file would you move the code of PageViewToolBar?
  
  
  Because it’s now a normal toolbar (defined in part.rc), the actions just need to be constructed and connected at some time. And we need some code to show/hide that toolbar when annotations are activated/deactivated, just like the PageViewToolBar is shown/hidden now.
  
  So why do we need this class (PageViewToolBar) at all? It is just convenient, because it bundles all the annotation actions (`ac->addAction(annArrow)` and so on) and their slots and some more. But it is not a toolbar itself. Maybe AnnotationToolBarController is a more accurate name. And AnnotationToolBarController would be at the same place as PageViewToolBar was. Or do I understand something wrong?
  
  So I think the new class PageViewToolBar is fine (I have looked at the code now), just the name is misleading now.

REPOSITORY
  R223 Okular

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

To: simgunz, #okular
Cc: anthonyfieroni, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel, joaonetto, tfella, darcyshen, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20190605/80c799b3/attachment.html>