ubuntu-touch-coreapps-reviewers team mailing list archive

[Nicholas Skaggs <nskaggs@xxxxxxxxxx>]

Review: Needs Fixing

Some suggested changes.

Diff comments:

> === modified file 'NewEvent.qml'
> --- NewEvent.qml	2015-03-18 23:56:25 +0000
> +++ NewEvent.qml	2015-04-19 01:45:37 +0000
> @@ -215,6 +215,12 @@
>                  event.setDetail(audibleReminder);
>              }
>              event.collectionId = calendarsOption.model[calendarsOption.selectedIndex].collectionId;
> +
> +            var comment = event.detail(Detail.Comment);
> +            if(comment && comment.comment === "X-CAL-DEFAULT-EVENT") {
> +                event.removeDetail(comment);
> +            }
> +
>              model.saveItem(event);
>              pageStack.pop();
>              root.eventAdded(event);
> 
> === modified file 'TimeLineBase.qml'
> --- TimeLineBase.qml	2015-01-22 21:17:51 +0000
> +++ TimeLineBase.qml	2015-04-19 01:45:37 +0000
> @@ -36,11 +36,18 @@
>      MouseArea {
>          anchors.fill: parent
>          objectName: "mouseArea"
> +
> +        onClicked: {
> +            var selectedDate = new Date(day);
> +            var hour = parseInt(mouseY / hourHeight);
> +            selectedDate.setHours(hour)
> +            createOrganizerEvent(selectedDate);
> +        }
> +
>          onPressAndHold: {
>              var selectedDate = new Date(day);
>              var hour = parseInt(mouseY / hourHeight);
>              selectedDate.setHours(hour)
> -            //pageStack.push(Qt.resolvedUrl("NewEvent.qml"), {"date":selectedDate, "model":eventModel});
>              createOrganizerEvent(selectedDate);
>          }
>  
> @@ -71,6 +78,7 @@
>          event.startDateTime = startDate;
>          event.endDateTime = endDate;
>          event.displayLabel = i18n.tr("Untitled");
> +        event.setDetail(Qt.createQmlObject("import QtOrganizer 5.0; Comment{ comment: 'X-CAL-DEFAULT-EVENT'}", event,"TimeLineBase.qml"));
>          model.saveItem(event);
>      }
>  
> @@ -90,7 +98,12 @@
>      }
>  
>      function showEventDetails(event) {
> -        pageStack.push(Qt.resolvedUrl("EventDetails.qml"), {"event":event,"model":model});
> +        var comment = event.detail(Detail.Comment);
> +        if(comment && comment.comment === "X-CAL-DEFAULT-EVENT") {
> +            pageStack.push(Qt.resolvedUrl("NewEvent.qml"),{"event": event, "model":model});
> +        } else {
> +            pageStack.push(Qt.resolvedUrl("EventDetails.qml"), {"event":event,"model":model});
> +        }
>      }
>  
>      WorkerScript {
> 
> === modified file 'TimeLineBaseComponent.qml'
> --- TimeLineBaseComponent.qml	2015-02-19 13:50:29 +0000
> +++ TimeLineBaseComponent.qml	2015-04-19 01:45:37 +0000
> @@ -63,8 +63,9 @@
>          var today = DateExt.today();
>          var startOfWeek = today.weekStart(Qt.locale().firstDayOfWeek);
>          var weekDay = today.getDay();
> +
>          if( startOfWeek.isSameDay(startDay) && weekDay > 2) {
> -            timeLineView.contentX = (weekDay * timeLineView.delegateWidth);
> +            timeLineView.contentX = (weekDay * timeLineView.delegateWidth);            
>              if( timeLineView.contentX  > (timeLineView.contentWidth - timeLineView.width) ) {
>                  timeLineView.contentX = timeLineView.contentWidth - timeLineView.width
>              }
> @@ -182,6 +183,11 @@
>                      }
>                  }
>  
> +                onContentWidthChanged: {
> +                    scrollToCurrentTime();
> +                    scrollTocurrentDate();
> +                }
> +
>                  clip: true
>  
>                  TimeLineBackground{}
> 
> === modified file 'WeekView.qml'
> --- WeekView.qml	2015-02-26 16:44:43 +0000
> +++ WeekView.qml	2015-04-19 01:45:37 +0000
> @@ -107,13 +107,6 @@
>                      startDay: firstDay.addDays( weekViewPath.indexType(index) * 7)
>                      keyboardEventProvider: weekViewPath
>  
> -                    Component.onCompleted: {
> -                        if(weekViewPage.isCurrentPage){
> -                            timeLineView.scrollToCurrentTime();
> -                            timeLineView.scrollTocurrentDate();
> -                        }
> -                    }
> -
>                      onIsActiveChanged: {
>                          timeLineView.scrollTocurrentDate();
>                      }
> 
> === modified file 'tests/autopilot/calendar_app/tests/test_monthview.py'
> --- tests/autopilot/calendar_app/tests/test_monthview.py	2015-04-06 17:07:11 +0000
> +++ tests/autopilot/calendar_app/tests/test_monthview.py	2015-04-19 01:45:37 +0000
> @@ -69,6 +69,8 @@
>          local = self.app.main_view.to_local_date(
>              self.month_view.currentMonth.datetime)
>          today = datetime.now()
> +        print(local)

you should make these logger.debug() if you'd like to see this information during the test.

> +        print(today)
>  
>          self.assertThat(lambda: local.day,
>                          Eventually(Equals(today.day)))
> 
> === modified file 'tests/autopilot/calendar_app/tests/test_weekview.py'
> --- tests/autopilot/calendar_app/tests/test_weekview.py	2015-04-10 18:50:59 +0000
> +++ tests/autopilot/calendar_app/tests/test_weekview.py	2015-04-19 01:45:37 +0000
> @@ -163,6 +163,10 @@
>          expected_month = dayStart.month
>          expected_year = dayStart.year
>  
> +        timeLineBase = self.week_view._get_timeline_base()
> +        timeline = timeLineBase.select_single(objectName="timelineview")
> +        while (timeline.contentX != 0):

while loops like this can cause the tests to never finish. You are not directly modifying this variable (timeline.contentX) so it can loop forever. This should be re-worked to prevent an infinite loop.

> +            self.app.main_view.swipe_view(-1, self.week_view)
>          self.app.pointing_device.click_object(day_to_select)
>  
>          # Check that the view changed from 'Week' to 'Day'
> @@ -172,6 +176,10 @@
>          # Check that the 'Day' view is on the correct/selected day.
>          selected_date = \
>              self.app.main_view.get_day_view().get_selected_day().startDay
> +
> +        print(expected_day)
> +        print(selected_date)
> +
>          self.assertThat(expected_day, Equals(selected_date.day))
>          self.assertThat(expected_month, Equals(selected_date.month))
>          self.assertThat(expected_year, Equals(selected_date.year))
> 


-- 
https://code.launchpad.net/~pkunal-parmar/ubuntu-calendar-app/TapToCreateEvent/+merge/254175
Your team Ubuntu Calendar Developers is subscribed to branch lp:ubuntu-calendar-app.