[Kde-pim] Review Request: Remove duplicated code in KOAgendaView::changeIncidenceDisplayAdded()

Sergio Martins iamsergio at gmail.com
Sun Feb 8 19:57:02 GMT 2009



> On 2009-02-08 11:38:11, Kevin Krammer wrote:
> > trunk/KDE/kdepim/korganizer/views/agendaview/koagendaview.cpp, line 1324
> > <http://reviewboard.kde.org/r/38/diff/2/?file=357#file357line1324>
> >
> >     since this was an if-then-else construct before, have you checked it wasn't there to have updateEventIndicators() between removeIncidence() and changeIncidenceDisplayAdded() and not after the latter?

yes, it doesn't even work if it's before changeIncidenceDisplayAdded().

Try this:
1. Open korganizer (unpatched) in week view
2. Scroll all up until only 00h-9am (9, 10, 11 will do) is visible.
3. Make a new event and set it's time to 22h-23h (so that the event isn't visible).

You won't see any green arrows (event indicators) unless you refresh the whole view.

I should have posted this little change in a separate review.


- Sergio


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


On 2009-02-08 11:08:40, Sergio Martins wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/38/
> -----------------------------------------------------------
> 
> (Updated 2009-02-08 11:08:40)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> This patch makes KOAgendaView::changeIncidenceDisplayAdded() share code with KOAgendaView::fillAgenda().
> 
> changeIncidenceDisplayAdded() has some bugs displaying multi-day recurrent events and multi day events whose start isn't visible, these bugs 
> were all fixed in fillAgenda recently.
> 
> Both these functions have the same goal, the difference is that changeIncidenceDisplayAdded only shows one incidence, fillAgenda shows them all, so I created a new private function, displayIncidence(), which is shared by both functions and implements all the recurrence/multi-day logic.
> 
> 
> This addresses bugs 99634 and 108443.
>     https://bugs.kde.org/show_bug.cgi?id=99634
>     https://bugs.kde.org/show_bug.cgi?id=108443
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepim/korganizer/views/agendaview/koagendaview.h 923377 
>   trunk/KDE/kdepim/korganizer/views/agendaview/koagendaview.cpp 923377 
> 
> Diff: http://reviewboard.kde.org/r/38/diff
> 
> 
> Testing
> -------
> 
> Tested with recurring events, recurring multi-day events, normal events, all day events, some of them without a visible start.
> 
> 
> Thanks,
> 
> Sergio
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/



More information about the kde-pim mailing list