Review Request: Port the Calculator applet to QML

[Mark Gaiser]

> On Nov. 6, 2012, 4:34 p.m., Mark Gaiser wrote:
> > applets/calculator/package/contents/ui/calculator.qml, lines 89-177
> > <http://git.reviewboard.kde.org/r/107001/diff/4/?file=93822#file93822line89>
> >
> >     (personal preference) Move the javascript functions to a javascript file.
> >     
> >     Then include it like so:
> >     import "helper.js" as Helper
> >     
> >     Then use it like so:
> >     Helper.functionName()
> >     
> >     Again, just my preference. Feel free to ignore it :)
> 
> Romário Rios wrote:
>     As these aren't used anywhere else, I don't see how's that necessary.

Yes, you are right. But it just looks cleaner in the QML file when all the code in there is actual QML with no javascript in it. It's fully up to you!


> On Nov. 6, 2012, 4:34 p.m., Mark Gaiser wrote:
> > applets/calculator/package/contents/ui/calculator.qml, lines 235-240
> > <http://git.reviewboard.kde.org/r/107001/diff/4/?file=93822#file93822line235>
> >
> >     Again my personal preference, but it cleans up your code. So here it is.
> >     
> >     I would put the button in a custom component since you are setting the width and height to the same value every time.
> >     
> >     // CustomButton.qml
> >     PlasmaComponents.Button {
> >         width: buttonWidth;
> >         height: buttonHeight;
> >     }
> >     
> >     Then in this case you call:
> >     CustomButton {
> >         text: "-"
> >         onClicked: setOperator("/")
> >     }
> >     
> >     The same for the other buttons. Seems cleaner to me..
> 
> Romário Rios wrote:
>     A repeater seems simpler to me in this case.

Well, he tried (in the mailing list) and a repeater gets a bit tricky when you want different components. Most of them are buttons, but there are a few empty placeholders as well. How do you suggest to do that in a repeater?

What he can do is repeat till the placeholders and continue manually from there. Even then he gets into trouble with the text and onClicked stuff.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107001/#review21491
-----------------------------------------------------------


On Nov. 6, 2012, 2:54 a.m., Romário Rios wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107001/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2012, 2:54 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> This diff replaces the C++ Calculator applet by its QMLfied version, which seems to be feature-par with the latter.
> 
> 
> Diffs
> -----
> 
>   applets/calculator/CMakeLists.txt 732145c 
>   applets/calculator/calculator.h f7339be 
>   applets/calculator/calculator.cpp 6bc3ddc 
>   applets/calculator/package/contents/ui/calculator.qml PRE-CREATION 
>   applets/calculator/package/metadata.desktop PRE-CREATION 
>   applets/calculator/plasma-applet-calculator.desktop 0760729 
> 
> Diff: http://git.reviewboard.kde.org/r/107001/diff/
> 
> 
> Testing
> -------
> 
> Tested as both applet and popupapplet, with the Air theme and the Oxygen theme. It seems to work fine.
> 
> 
> Thanks,
> 
> Romário Rios
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20121106/51f5e035/attachment-0001.html>