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