Merge lp:~larochelle-brian/ubuntu-calendar-app/30MinIncrementsHackdays into lp:ubuntu-calendar-app

[brian larochelle]

Added logic to NewEvent.qml to choose a default of either 0 or 30 for minutes for the TimePicker. If minutes are past 30, an hour is added.

Added logic to NewEvent.qml to choose a default of either 0 or 30 for minutes for the TimePicker. If minutes are past 30, an hour is added.

[Kunal Parmar (pkunal-parmar)]

8 + startTime.defaultHour = (startDate.getMinutes() < 30) ? startDate.getHours() : startDate.getHours() + 1
19 + startTime.defaultMinute = (startDate.getMinutes() < 30) ? 30 : 0
20 + var popupObj = PopupUtils.open(timePicker,root,{"hour": startTime.defaultHour,"minute":startTime.defaultMinute});

Can we move above code to separate function ?

[brian larochelle (larochelle-brian)]

sure.

I moved the logic to functions off the root of NewEvent {} and changed the ternary statements to if statements to improve readability.

[Kunal Parmar (pkunal-parmar)]

Code looks great, Thanks.

I will test and approve it.

[Kunal Parmar (pkunal-parmar)]

8 - var popupObj = PopupUtils.open(timePicker,root,{"hour": endDate.getHours(),"minute":endDate.getMinutes()});
39 + var popupObj = PopupUtils.open(timePicker,root,{"hour": root.hourForPickerFromDate(startDate),"minute":root.minuteForPickerFromDate(startDate)});

looks copy/paste error.

You are using startDate, instead of endDate

review: Needs Fixing

[brian larochelle (larochelle-brian)]

Opps. I shouldn't have let that slip by. Thats what I get for trying to sneak this in at work... resubmitting.
thanks for the review.

[brian larochelle (larochelle-brian)]

Sorry again. I made another error. I'll take a little more time for this. Currently at work and was rushing to fix this.

review: Needs Fixing

[Kunal Parmar (pkunal-parmar)]

:),

One more thing, that I am not sure of but.

Currently you are setting incremental value to time picker.

But do we need to set incremental time to start and end time label as well on construction ?

[brian larochelle (larochelle-brian)]

I was wondering that myself. I didn't see a clear pro or con, so I left it as is. I kinda like the label getting the real time at construction, but that preference isn't based on much. I'd be willing to change it, if that is decided.

[lp:~larochelle-brian/ubuntu-calendar-app/30MinIncrementsHackdays]

225. By brian larochelle on 2014-03-27

Fix copy/paste error.
Increment the endDate by 30 instread of 10

[brian larochelle (larochelle-brian)]

Fixed copy/paste error.

also noticed endDate should have its minutes incremented by 30 instead of 10.
17 - endDate.setMinutes( endDate.getMinutes() + 10)
18 + endDate.setMinutes( endDate.getMinutes() + 30)

review: Needs Resubmitting

[Kunal Parmar (pkunal-parmar)]

Thanks :), Looks great,
I will test once more and approve.

[Kunal Parmar (pkunal-parmar)]

looks good to me

review: Approve

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

review: Approve (continuous-integration)

=== modified file 'NewEvent.qml'
--- NewEvent.qml	2014-02-19 11:39:13 +0000
+++ NewEvent.qml	2014-03-27 14:56:26 +0000
@@ -33,7 +33,7 @@
         // If endDate is setted by argument we have to not change it
         if (typeof(endDate) === 'undefined') {
             endDate = new Date(date)
-            endDate.setMinutes( endDate.getMinutes() + 10)
+            endDate.setMinutes( endDate.getMinutes() + 30)
         }
         internal.eventModel = GlobalModel.globalModel();
 
@@ -52,7 +52,7 @@
         event = Qt.createQmlObject("import QtOrganizer 5.0; Event { }", Qt.application,"NewEvent.qml");
         startDate = new Date(date)
         endDate = new Date(date)
-        endDate.setMinutes( endDate.getMinutes() + 10)
+        endDate.setMinutes( endDate.getMinutes() + 30)
 
         startTime.text = Qt.formatDateTime(startDate, "dd MMM yyyy hh:mm");
         endTime.text = Qt.formatDateTime(endDate, "dd MMM yyyy hh:mm");
@@ -159,6 +159,20 @@
         }
     }
 
+    // Calucate default hour for start and end time on event
+    function hourForPickerFromDate(date) {
+        if(date.getMinutes() < 30)
+            return date.getHours()
+        return date.getHours() + 1
+    }
+
+    // Calucate default minute for start and end time on event
+    function minuteForPickerFromDate(date) {
+        if(date.getMinutes() < 30)
+            return 30
+        return 0
+    }
+
     width: parent.width
     height: parent.height
 
@@ -269,7 +283,7 @@
                             anchors.fill: parent
                             onClicked: {
                                 internal.clearFocus()
-                                var popupObj = PopupUtils.open(timePicker,root,{"hour": startDate.getHours(),"minute":startDate.getMinutes()});
+                                var popupObj = PopupUtils.open(timePicker,root,{"hour": root.hourForPickerFromDate(startDate),"minute":root.minuteForPickerFromDate(startDate)});
                                 popupObj.accepted.connect(function(startHour, startMinute) {
                                     var newDate = startDate;
                                     newDate.setHours(startHour, startMinute);
@@ -294,7 +308,7 @@
                             anchors.fill: parent
                             onClicked: {
                                 internal.clearFocus()
-                                var popupObj = PopupUtils.open(timePicker,root,{"hour": endDate.getHours(),"minute":endDate.getMinutes()});
+                                var popupObj = PopupUtils.open(timePicker,root,{"hour": root.hourForPickerFromDate(endDate),"minute":root.minuteForPickerFromDate(endDate)});
                                 popupObj.accepted.connect(function(startHour, startMinute) {
                                     var newDate = endDate;
                                     newDate.setHours(startHour, startMinute);