ubuntu-touch-coreapps-reviewers team mailing list archive

[Niklas Wenzel <nikwen.developer@xxxxxxxxx>]

Review: Needs Fixing

Looks fine to me. I just have one suggestion which I've added to the diff.
That being said, I'll happily approve this if you apply that small change. :)

Diff comments:

> === modified file 'app/alarm/AlarmPage.qml'
> --- app/alarm/AlarmPage.qml	2014-10-28 10:54:45 +0000
> +++ app/alarm/AlarmPage.qml	2015-03-05 12:57:48 +0000
> @@ -140,7 +140,12 @@
>  
>      Loader {
>          id: emptyStateLoader
> -        anchors.centerIn: parent
> +        anchors {
> +            left: parent.left
> +            right: parent.right
> +            margins: units.gu(2)
> +            verticalCenter: parent.verticalCenter
> +        }
>          active: alarmModel ? alarmModel.count === 0 : true
>          Component.onCompleted: {
>              setSource(Qt.resolvedUrl("../components/EmptyState.qml"),
> 
> === modified file 'app/components/EmptyState.qml'
> --- app/components/EmptyState.qml	2014-10-16 14:24:25 +0000
> +++ app/components/EmptyState.qml	2015-03-05 12:57:48 +0000
> @@ -33,6 +33,7 @@
>      property alias subTitle: emptySublabel.text
>  
>      height: childrenRect.height
> +    width: parent.width
>  
>      Icon {
>          id: emptyIcon
> @@ -44,16 +45,19 @@
>  
>      Label {
>          id: emptyLabel

Why not set wrapMode here, too? Just to be sure, in case translations get too long or the screen is too small. ;)

> +        fontSize: "large"
> +        font.bold: true
> +        width: parent.width
>          anchors.top: emptyIcon.bottom
>          anchors.topMargin: units.gu(5)
> -        anchors.horizontalCenter: parent.horizontalCenter
> -        fontSize: "large"
> -        font.bold: true
> +        horizontalAlignment: Text.AlignHCenter
>      }
>  
>      Label {
>          id: emptySublabel
> +        width: parent.width
> +        wrapMode: Text.WordWrap
>          anchors.top: emptyLabel.bottom
> -        anchors.horizontalCenter: parent.horizontalCenter
> +        horizontalAlignment: Text.AlignHCenter
>      }
>  }
> 
> === modified file 'debian/changelog'
> --- debian/changelog	2015-02-26 21:21:41 +0000
> +++ debian/changelog	2015-03-05 12:57:48 +0000
> @@ -9,6 +9,7 @@
>    * Fixed Day-of-Week picker in alarms not respecting user locale (LP: #1372545)
>    * Fixed predefined cities and countries not being translatable in the timezone
>      selection dialog (LP: #1354466)
> +  * Fixed empty state description not wrapping (LP: #1428165)
>  
>    [Brendan Donegan]
>    * Fixed AP failure by waiting for the bottom edge tip visible property to be true
> 
> === modified file 'po/com.ubuntu.clock.pot'
> --- po/com.ubuntu.clock.pot	2015-02-27 12:15:58 +0000
> +++ po/com.ubuntu.clock.pot	2015-03-05 12:57:48 +0000
> @@ -8,7 +8,7 @@
>  msgstr ""
>  "Project-Id-Version: \n"
>  "Report-Msgid-Bugs-To: \n"
> -"POT-Creation-Date: 2015-02-27 13:15+0100\n"
> +"POT-Creation-Date: 2015-03-05 13:52+0100\n"
>  "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
>  "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
>  "Language-Team: LANGUAGE <LL@xxxxxx>\n"
> @@ -48,11 +48,11 @@
>  msgid "Select All"
>  msgstr ""
>  
> -#: ../app/alarm/AlarmPage.qml:149
> +#: ../app/alarm/AlarmPage.qml:154
>  msgid "No saved alarms"
>  msgstr ""
>  
> -#: ../app/alarm/AlarmPage.qml:150
> +#: ../app/alarm/AlarmPage.qml:155
>  msgid "Tap the + icon to add an alarm"
>  msgstr ""
>  
> 


-- 
https://code.launchpad.net/~nik90/ubuntu-clock-app/fix-empty-state-wrap/+merge/251924
Your team Ubuntu Clock Developers is subscribed to branch lp:ubuntu-clock-app.