← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

Re: [Merge] lp:~gang65/ubuntu-calculator-app/ubuntu-calculator-app-multiline-support into lp:ubuntu-calculator-app

 

Review: Needs Fixing

Works good, but I left an inline comment. 

Also, there isn't a way to use a single component editing proprierties instead of having two different Rows?

Diff comments:

> 
> === modified file 'app/ui/Screen.qml'
> --- app/ui/Screen.qml	2015-11-24 13:43:38 +0000
> +++ app/ui/Screen.qml	2015-11-28 21:27:19 +0000
> @@ -52,18 +52,20 @@
>      }
>  
>      color: "white"
> +    height: units.gu(7) + (mainView.isLandscapeView ? 0 : units.gu(3.7))
>      Column {
>          anchors.fill: parent
>          Row {
>              id: creationDateRow
>              width: parent.width
> +            height: units.gu(1.8)
>              anchors.right: parent.right
> -            spacing: units.gu(1)
>  
>              layoutDirection: Qt.RightToLeft
>  
>              Text {
>                  id: creationTimeText
> +                height: units.gu(1.8)

Please, use anchors { top: parent.top, bottom: parent.bottom }, it's more efficent that settings the height

>                  color: UbuntuColors.darkGrey
>                  text: formatDate(model.date)
>                  textFormat: Text.PlainText


-- 
https://code.launchpad.net/~gang65/ubuntu-calculator-app/ubuntu-calculator-app-multiline-support/+merge/278897
Your team Ubuntu Calculator Developers is subscribed to branch lp:ubuntu-calculator-app.


References