-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118668/#review60307
-----------------------------------------------------------


No comments on most of the code change since that looks OK to me :) Besides 
setToday.
Just a friendly explanation why i did it this way and why i (back then) thought 
it would be sufficient.

I knew there was going to be a usecase where users of the class would want to 
know the current day, would want to change it and move back to it. My reasoning 
for not implementing functionality for that is because startDate could handle 
it all.
- Want to jump to a date? startDate = new Date(YYYY, MM, DD)
- Want to jump back to the current date? startDate = new Date()
- Want to know the current day? new Date() :)

But i do see now that my naming back then (startDate) is confusing when it 
allows you to do multiple things. I'm sorry for the hassle you must have went 
through.

Lastly setToday. I don't understand that one. today() should always return a 
QDate::currentDate(), right? Why should there be a setter for it? If i 
understand it correctly you only need this because of the dataengine providing 
date updates, right? If that's the case then i don't see why i needs to be in 
this class.. It should be in the QML side as a custom property like so:

Calendar {
  id: cal
  property date dateEngineDate: somethingFromTheDataEngine
  onDateEngineDateChanged: {
    // do your magic.. Probably the most part of your current setToday function
  }
}

Or i am completely wrong, quite possibly :)

- Mark Gaiser


On June 17, 2014, 8:44 a.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118668/
> -----------------------------------------------------------
> 
> (Updated June 17, 2014, 8:44 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> Basically splits the Calendar::m_startDate into 'today' and 'displayedDate', 
> where displayedDate is the date that is displayed (it controls the days model 
> etc) and can be manipulated by the user by eg. changing months in the 
> plasmoid, and today is the current day, populated by our dataengine (which 
> means it auto-updates with no need for a timer). This allows for greater 
> flexibility and things like "Go back to today" when eg. the plasmoid is 
> hidden or when the user have browsed too far in the calendar and just wants 
> to get back to today (the button to do that pending). Also this fixes a 
> problem where the time dataengine is being polled every 30secs for the clock 
> and would reset the calendar view as the startDate is currently bound to the 
> dataengine and the view resets on that change.
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/calendar/calendar.h fd2c534 
>   src/declarativeimports/calendar/calendar.cpp 4225579 
>   src/declarativeimports/calendar/qml/MonthMenu.qml 89e9dc2 
>   src/declarativeimports/calendar/qml/MonthView.qml eee850d 
> 
> Diff: https://git.reviewboard.kde.org/r/118668/diff/
> 
> 
> Testing
> -------
> 
> All works properly
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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

Reply via email to