'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] 