← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

Re: [Merge] lp:~fboucault/ubuntu-calculator-app/startup_time into lp:ubuntu-calculator-app

 

Review: Needs Fixing

Wow! It is amazing work!
I'm impressed. 
The startup time was improved dramatically.

I found some nitpick which will be good to resolve before final merge.

Thanks a lot.


Diff comments:

> 
> === modified file 'app/ubuntu-calculator-app.qml'
> --- app/ubuntu-calculator-app.qml	2016-01-18 23:40:25 +0000
> +++ app/ubuntu-calculator-app.qml	2016-08-09 12:38:05 +0000
> @@ -264,18 +270,11 @@
>  
>          PageWithBottomEdge {
>              id: calculatorPage
> -            title: i18n.tr("Calculator")

Without that title, for Desktop title is empty.
To reproduce it run calculator on Desktop with command:
 $ qmlscene ubuntu-calculator-app.qml

>              anchors.fill: parent
>              visible: false
>  
>              bottomEdgeTitle: i18n.tr("Favorite")
> -
> -            bottomEdgePageComponent: FavouritePage {
> -                anchors.fill: parent
> -
> -                title: i18n.tr("Favorite")
> -            }
> -
> +            bottomEdgePageSource: "ui/FavouritePage.qml"
>              bottomEdgeEnabled: textInputField.visible
>  
>              state: visualModel.isInSelectionMode ? "selection" : "default"
> 
> === modified file 'app/ui/KeyboardButton.qml'
> --- app/ui/KeyboardButton.qml	2015-11-09 14:18:18 +0000
> +++ app/ui/KeyboardButton.qml	2016-08-09 12:38:05 +0000
> @@ -19,11 +19,10 @@
>  import QtQuick 2.4
>  import Ubuntu.Components 1.3
>  
> -AbstractButton {
> +MouseArea {
>      id: buttonRect
>      objectName: modelname + "Button"
>  
> -    property real baseSize: 1
>      property alias text: buttonText.text
>      property string buttonColor: "#eeeeee"
>      property string pressedColor: "#E2E1E4"

If magic numbers is causing performance boost then please remove old properties (pressedColor and buttonColor)



-- 
https://code.launchpad.net/~fboucault/ubuntu-calculator-app/startup_time/+merge/302403
Your team Ubuntu Calculator Developers is subscribed to branch lp:ubuntu-calculator-app.


References