davidhurka marked an inline comment as done. davidhurka added a comment.
In D21195#477747 <https://phabricator.kde.org/D21195#477747>, @ngraham wrote: > I would recommend making the toolbar button show the text of the actual color change mode that's currently active, much like how the selection tools toolbar button currently does. Otherwise you won't know which mode it will invoke. Implemented that. How do you like it? INLINE COMMENTS > davidhurka wrote in pageview.h:75 > Adding a link to Part is neccessary, because Part controls m_embedMode, which > is used to access the configuration dialog. > > However, the whole Change Colors menu is probably better in Part instead of > PageView. Currently, every PageView creates a new menu, and with multible > tabs their checked states get out of sync. > > m_embedMode gives another question: should I check m_embedMode before > creating this menu? In the print preview mode, Change Colors does not make > much sense. In the meantime I learned more about this. The Color Mode menu will stay here in PageView::setupViewerActions(), because it is needed only in viewer modes, and is page view related. m_embedMode is not needed, that’s handled by Part. But I still don’t like the parameter part, any comments? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D21195 To: davidhurka, #okular, #vdg Cc: GB_2, davidre, aacid, ngraham, okular-devel, fbampaloukas, joaonetto, tfella, darcyshen
