'Re: Review Request: Master Notes for KPresenter'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: Review Request: Master Notes for KPresenter
From:       "Fredy Yanardi" <fyanardi () gmail ! com>
Date:       2009-11-17 17:39:21
Message-ID: 20091117173921.27424.87261 () localhost
[Download RAW message or body]

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

(Updated 2009-11-17 17:39:21.462958)

Review request for KOffice.

Changes
-------

Updated patch:
* Problem in select all is because KoPAView::editSelectAll() gets all shapes directly from \
current active page's child shapes. To overcome this, I propose a new virtual method in \
KoPAViewMode to get the child shapes of the active page related to this view mode. Currently I \
                name it KoPAViewMode::activePageChildShapes().
* Fix add/remove shapes in Notes / Notes Master view mode
* Fix showing the master shapes when in Notes view mode
* Don't crash when saving notes / notes master without text / thumbnail shape

Summary
-------

This is a patch to enable master notes for KPresenter. The patch is still work in progress, but \
all basic functionality and all necessary changes are there. What left is saving the master \
notes (I think). Here I list all proposed changes for kopageapp and kpresenter:

For kopageapp, there are some changes in KoPAView and KoPAViewMode classes to support the \
master notes and to allow more flexibility for similar feature in the future (i.e: view mode \
that has normal and master mode): 1. Additional parameter [bool master] for \
KoPAViewMode::setViewMode, to allow activating a view mode and directly enable/disable master \
mode for that view mode. So now the function signature is: void KoPAViewMode::setViewMode( \
KoPAViewMode *mode, bool master ); 2. Additional parameter [bool updateShapeManager] for \
KoPAViewMode::setActivePage. Currently if this method is called, the shape manager of the \
canvas is updated with the page's shapes. But this is redundant for KPrViewModeNotes, since it \
needs to update the shape manager with different set of shapes. The additional parameter will \
tell the function whether to update the shape manager of not. So, the proposed function \
signature is: void KoPAViewMode::setActivePage( KoPAPageBase *page, bool updateShapeManager ); \
3. Additional parameter [bool master] for KoPAViewMode::activate to facilitate[1]. The function \
signature becomes: void KoPAViewMode::activate( KoPAViewMode *previousViewMode, bool master ) \
4. Additional function KoPAView::setViewModeSavedPage( KoPAPageBase *page ) and \
KoPAView::viewModeSavedPage(). The reason for this is because we need to save the normal page \
(KoPAPage) when we go to master mode, so that if we go back to normal mode again, that saved \
page can be activated. Previously only one view mode can go to master mode, hence the KoPAPage \
is saved inside the KoPAViewModeNormal. But now there are two view modes that can go to master \
mode, hence there must be a central place to set and get the saved page. 5. Remove \
doUpdateActivePage function from KoPAView, and do the updating of the active page in each view \
                mode's updateActivePage instead. There are two main reasons I propose this \
                change:
   * The implementation for updating active page is different in each view mode, and it is only \
                used in KoPAViewModeNormal.
   * Too many routing for this doUpdateActivePage. Currently for updating active page of the \
KoPAViewModeNormal is: KoPAView::updateActivePage() -> KoPAViewMode::updateActivePage()-> \
KoPAView::doUpdateActivePage().  (This is not directly related to master notes, just to \
simplify and clean up a bit)

And for KPresenter itself:
1. Split KPrNotes into three classes: KPrNotesBase, KPrMasterNotes, and KPrNotes. KPrNotes gets \
the thumbnail shape, text shape and page layout from KPrMasterNotes. 2. Some changes in \
KPrViewModeNotes to support displaying notes and masternotes shapes.

Any critics, comments or suggestions are greatly appreciated :)

Diffs (updated)
-----

  /trunk/koffice/kpresenter/part/CMakeLists.txt 1050110 
  /trunk/koffice/kpresenter/part/KPrMasterNotes.h PRE-CREATION 
  /trunk/koffice/kpresenter/part/KPrMasterNotes.cpp PRE-CREATION 
  /trunk/koffice/kpresenter/part/KPrMasterPage.h 1050110 
  /trunk/koffice/kpresenter/part/KPrMasterPage.cpp 1050110 
  /trunk/koffice/kpresenter/part/KPrNotes.h 1050110 
  /trunk/koffice/kpresenter/part/KPrNotes.cpp 1050110 
  /trunk/koffice/kpresenter/part/KPrNotesBase.h PRE-CREATION 
  /trunk/koffice/kpresenter/part/KPrNotesBase.cpp PRE-CREATION 
  /trunk/koffice/kpresenter/part/KPrView.h 1050110 
  /trunk/koffice/kpresenter/part/KPrView.cpp 1050110 
  /trunk/koffice/kpresenter/part/KPrViewModeNotes.h 1050110 
  /trunk/koffice/kpresenter/part/KPrViewModeNotes.cpp 1050110 
  /trunk/koffice/kpresenter/part/KPrViewModePresentation.h 1050110 
  /trunk/koffice/kpresenter/part/KPrViewModePresentation.cpp 1050110 
  /trunk/koffice/kpresenter/part/KPrViewModePreviewPageEffect.h 1050110 
  /trunk/koffice/kpresenter/part/KPrViewModePreviewPageEffect.cpp 1050110 
  /trunk/koffice/kpresenter/part/kpresenter.rc 1050110 
  /trunk/koffice/libs/kopageapp/KoPAView.h 1050110 
  /trunk/koffice/libs/kopageapp/KoPAView.cpp 1050110 
  /trunk/koffice/libs/kopageapp/KoPAViewMode.h 1050110 
  /trunk/koffice/libs/kopageapp/KoPAViewMode.cpp 1050110 
  /trunk/koffice/libs/kopageapp/KoPAViewModeNormal.h 1050110 
  /trunk/koffice/libs/kopageapp/KoPAViewModeNormal.cpp 1050110 

Diff: http://reviewboard.kde.org/r/2167/diff

Testing
-------

Thanks,

Fredy

_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel

[prev in list] [next in list] [prev in thread] [next in thread] 