ubuntu-touch-coreapps-reviewers team mailing list archive

[Nicholas Skaggs <nskaggs@xxxxxxxxxx>]

Review: Approve

Minor comments. Feel free to merge once addressed.

Diff comments:

> === modified file 'MonthComponent.qml'
> --- MonthComponent.qml	2014-11-29 09:40:53 +0000
> +++ MonthComponent.qml	2015-03-05 22:04:24 +0000
> @@ -148,6 +148,7 @@
>  
>              Row{
>                  id: dayLabelRow
> +                objectName: "dayLabelRow" + index
>  
>                  property int dayWidth: width / 7;
>  
> @@ -230,6 +231,7 @@
>  
>          Label{
>              id: weekDay
> +            objectName: "weekDay" + index
>              width: parent.dayWidth
>              property var day :Qt.locale().standaloneDayName(( Qt.locale().firstDayOfWeek + index), Locale.ShortFormat)
>              text: isYearView ? day.charAt(0) : day;
> 
> === modified file 'MonthView.qml'
> --- MonthView.qml	2015-02-18 19:27:20 +0000
> +++ MonthView.qml	2015-03-05 22:04:24 +0000
> @@ -50,6 +50,7 @@
>          ]
>  
>          contents: Label {
> +            objectName:"monthYearLabel"
>              fontSize: "x-large"
>              // TRANSLATORS: this is a time formatting string,
>              // see http://qt-project.org/doc/qt-5/qml-qtqml-date.html#details for valid expressions.
> 
> === modified file 'tests/autopilot/calendar_app/__init__.py'
> --- tests/autopilot/calendar_app/__init__.py	2015-02-26 21:11:03 +0000
> +++ tests/autopilot/calendar_app/__init__.py	2015-03-05 22:04:24 +0000
> @@ -435,10 +435,26 @@
>      def get_current_month(self):
>          return self.select_single('MonthComponent', isCurrentItem=True)
>  
> +    @autopilot.logging.log_action(logger.info)
>      def get_current_month_name(self):
>          month = self.get_current_month()
>          return month.currentMonth.datetime.strftime("%B")
>  
> +    @autopilot.logging.log_action(logger.info)
> +    def get_current_selected_day(self):
> +        today_days = self.select_many(
> +            'MonthComponentDateDelegate', isToday=True)
> +        for item in today_days:

Can you use a select_single here instead to select from today_days(isCurrentMonth=True)? I'm not sure you can or that's it better, but just commenting. No need to change it if it doesn't make sense.

> +            if item.isCurrentMonth:
> +                return item
> +
> +    @autopilot.logging.log_action(logger.info)

Ahh brilliant, you added logging. I'm guilty of forgetting this myself!

> +    def get_day_label(self, day):
> +        days_row = self.select_single(
> +            'QQuickRow', objectName='dayLabelRow0')
> +        return days_row.select_single(
> +            'Label', objectName='weekDay{}'.format(day))
> +
>  
>  class DayView(ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase):
>  
> 
> === modified file 'tests/autopilot/calendar_app/tests/test_monthview.py'
> --- tests/autopilot/calendar_app/tests/test_monthview.py	2014-09-04 16:04:37 +0000
> +++ tests/autopilot/calendar_app/tests/test_monthview.py	2015-03-05 22:04:24 +0000
> @@ -22,6 +22,7 @@
>  from testtools.matchers import Equals, NotEquals
>  
>  import math
> +import calendar
>  
>  from calendar_app.tests import CalendarAppTestCase
>  
> @@ -96,3 +97,55 @@
>  
>      def test_monthview_go_to_today_prev_year(self):
>          self._go_to_today(-12)
> +
> +    def test_current_day_month_and_year_is_selected(self):
> +        """
> +        By default, the month view shows the current day, month and year.
> +        """
> +        now = datetime.now()
> +        expected_month_name_year = now.strftime("%B %Y")
> +
> +        self.assertThat(self.app.main_view.get_month_year(self.month_view),
> +                        Equals(expected_month_name_year))
> +
> +        expected_day = str(int(now.strftime("%d")))
> +        selected_day = self.month_view.get_current_selected_day()
> +
> +        self.assertThat(selected_day.select_single('Label').text,

You can use assertEquals(a, b) here, I think it reads a bit cleaner.

> +                        Equals(expected_day))
> +
> +    def test_days_of_week_are_correct(self):
> +        """
> +        Verify that days of week are correct for the locale
> +        """
> +        first_week_day = calendar.day_abbr[calendar.firstweekday()]
> +        day_0_label = self.month_view.get_day_label(0).day
> +        day_1_label = self.month_view.get_day_label(1).day
> +        day_2_label = self.month_view.get_day_label(2).day
> +        day_3_label = self.month_view.get_day_label(3).day
> +        day_4_label = self.month_view.get_day_label(4).day
> +        day_5_label = self.month_view.get_day_label(5).day
> +        day_6_label = self.month_view.get_day_label(6).day
> +
> +        self.assertThat(day_0_label, Equals(first_week_day))
> +
> +        self.assertThat(

Same thing, just use assertEquals. I find myself using assertThat for everything too :-)

> +            calendar.day_abbr[calendar.MONDAY], Equals(day_0_label))
> +
> +        self.assertThat(
> +            calendar.day_abbr[calendar.TUESDAY], Equals(day_1_label))
> +
> +        self.assertThat(
> +            calendar.day_abbr[calendar.WEDNESDAY], Equals(day_2_label))
> +
> +        self.assertThat(
> +            calendar.day_abbr[calendar.THURSDAY], Equals(day_3_label))
> +
> +        self.assertThat(
> +            calendar.day_abbr[calendar.FRIDAY], Equals(day_4_label))
> +
> +        self.assertThat(
> +            calendar.day_abbr[calendar.SATURDAY], Equals(day_5_label))
> +
> +        self.assertThat(
> +            calendar.day_abbr[calendar.SUNDAY], Equals(day_6_label))
> 


-- 
https://code.launchpad.net/~carla-sella/ubuntu-calendar-app/current-day-month-year-selected/+merge/251387
Your team Ubuntu Calendar Developers is subscribed to branch lp:ubuntu-calendar-app.