Re: Review Request: Port the Calculator applet to QML

[Commit Hook]

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


This review has been submitted with commit 
08960f71610ffd869a7fafb495cfb24e2207bb0e by Luiz Romário Santana Rios to branch 
master.

- Commit Hook


On Nov. 6, 2012, 11:36 p.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, 11:36 p.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
> -
> 
>   CMakeLists.txt 732145c 
>   calculator.h f7339be 
>   calculator.cpp 6bc3ddc 
>   package/contents/ui/calculator.qml PRE-CREATION 
>   package/metadata.desktop PRE-CREATION 
>   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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Marco Martin]

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

Ship it!


Ship It!

- Marco Martin


On Nov. 6, 2012, 11:36 p.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, 11:36 p.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
> -
> 
>   CMakeLists.txt 732145c 
>   calculator.h f7339be 
>   calculator.cpp 6bc3ddc 
>   package/contents/ui/calculator.qml PRE-CREATION 
>   package/metadata.desktop PRE-CREATION 
>   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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Romário Rios]

> On Nov. 8, 2012, 9:33 a.m., Marco Martin wrote:
> > style is quite ok to me now.
> > functions could still be changed to
> > 
> > function foo()
> > {
> > ...
> > }
> 
> Aaron J. Seigo wrote:
> actually, the style was finally updated to:
> 
> function foo() {
> }
> 
> so no need to adjust this :)
> 
> Marco Martin wrote:
> aand ignore last comment, style guidelines ended up being
> function foo () {
>   ..
> }
> 
> inthe end. so the current code is ok

Then, as the hard freeze is scheduled for today, I'm gonna merge this later 
tonight if noone objects.


- Romário


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


On Nov. 6, 2012, 11:36 p.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, 11:36 p.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
> -
> 
>   CMakeLists.txt 732145c 
>   calculator.h f7339be 
>   calculator.cpp 6bc3ddc 
>   package/contents/ui/calculator.qml PRE-CREATION 
>   package/metadata.desktop PRE-CREATION 
>   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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Marco Martin]

> On Nov. 8, 2012, 9:33 a.m., Marco Martin wrote:
> > style is quite ok to me now.
> > functions could still be changed to
> > 
> > function foo()
> > {
> > ...
> > }
> 
> Aaron J. Seigo wrote:
> actually, the style was finally updated to:
> 
> function foo() {
> }
> 
> so no need to adjust this :)

aand ignore last comment, style guidelines ended up being
function foo () {
  ..
}

inthe end. so the current code is ok


- Marco


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


On Nov. 6, 2012, 11:36 p.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, 11:36 p.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
> -
> 
>   CMakeLists.txt 732145c 
>   calculator.h f7339be 
>   calculator.cpp 6bc3ddc 
>   package/contents/ui/calculator.qml PRE-CREATION 
>   package/metadata.desktop PRE-CREATION 
>   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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Aaron J. Seigo]

> On Nov. 8, 2012, 9:33 a.m., Marco Martin wrote:
> > style is quite ok to me now.
> > functions could still be changed to
> > 
> > function foo()
> > {
> > ...
> > }

actually, the style was finally updated to:

function foo() {
}

so no need to adjust this :)


- Aaron J.


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


On Nov. 6, 2012, 11:36 p.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, 11:36 p.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
> -
> 
>   CMakeLists.txt 732145c 
>   calculator.h f7339be 
>   calculator.cpp 6bc3ddc 
>   package/contents/ui/calculator.qml PRE-CREATION 
>   package/metadata.desktop PRE-CREATION 
>   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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Marco Martin]

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


style is quite ok to me now.
functions could still be changed to

function foo()
{
...
}

- Marco Martin


On Nov. 6, 2012, 11:36 p.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, 11:36 p.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
> -
> 
>   CMakeLists.txt 732145c 
>   calculator.h f7339be 
>   calculator.cpp 6bc3ddc 
>   package/contents/ui/calculator.qml PRE-CREATION 
>   package/metadata.desktop PRE-CREATION 
>   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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Mark Gaiser]

> On Nov. 7, 2012, 12:02 a.m., Mark Gaiser wrote:
> > Actually, i think you can get rid of all the onClicked: someAction in all 
> > the buttons since you are already catching those events in 
> > Keys.onSomeAction.

... nevermind this one as well.
Those onClicked are for when a user presses the button with a mouse. I probably 
should think twice before making the next reply in here ;)


- Mark


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


On Nov. 6, 2012, 11:36 p.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, 11:36 p.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
> -
> 
>   CMakeLists.txt 732145c 
>   calculator.h f7339be 
>   calculator.cpp 6bc3ddc 
>   package/contents/ui/calculator.qml PRE-CREATION 
>   package/metadata.desktop PRE-CREATION 
>   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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Mark Gaiser]

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


Actually, i think you can get rid of all the onClicked: someAction in all the 
buttons since you are already catching those events in Keys.onSomeAction.

- Mark Gaiser


On Nov. 6, 2012, 11:36 p.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, 11:36 p.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
> -
> 
>   CMakeLists.txt 732145c 
>   calculator.h f7339be 
>   calculator.cpp 6bc3ddc 
>   package/contents/ui/calculator.qml PRE-CREATION 
>   package/metadata.desktop PRE-CREATION 
>   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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: 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, line 231
> > 
> >
> > I guess all the string values need to be wrapped in i18nc("C") .. 
> > Someone else would need to confirm this since i don't know for sure. This 
> > is for all the parts where you use a string.
> 
> Romário Rios wrote:
> Every string displayed to the user you mean.
> 
> Anyway, is this necessary for the numbers?

Yes, the user visible ones. Don't know for the numbers.


- Mark


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


On Nov. 6, 2012, 11:36 p.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, 11:36 p.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
> -
> 
>   CMakeLists.txt 732145c 
>   calculator.h f7339be 
>   calculator.cpp 6bc3ddc 
>   package/contents/ui/calculator.qml PRE-CREATION 
>   package/metadata.desktop PRE-CREATION 
>   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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Romário Rios]

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

(Updated Nov. 6, 2012, 11:36 p.m.)


Review request for Plasma.


Changes
---

Resolved issues raised by Mark


Description
---

This diff replaces the C++ Calculator applet by its QMLfied version, which 
seems to be feature-par with the latter.


Diffs (updated)
-

  CMakeLists.txt 732145c 
  calculator.h f7339be 
  calculator.cpp 6bc3ddc 
  package/contents/ui/calculator.qml PRE-CREATION 
  package/metadata.desktop PRE-CREATION 
  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

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Romário Rios]

> On Nov. 6, 2012, 4:34 p.m., Mark Gaiser wrote:
> > applets/calculator/package/contents/ui/calculator.qml, lines 235-240
> > 
> >
> > 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.
> 
> Mark Gaiser wrote:
> 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 Gaiser wrote:
> .. ignore my comment above :p

Well, anyway, this is work for later IMO. It's this way because the original 
C++ applet created its buttons just like this (it didn't use a for loop or 
created a different class for the button).


> On Nov. 6, 2012, 4:34 p.m., Mark Gaiser wrote:
> > applets/calculator/package/contents/ui/calculator.qml, line 231
> > 
> >
> > I guess all the string values need to be wrapped in i18nc("C") .. 
> > Someone else would need to confirm this since i don't know for sure. This 
> > is for all the parts where you use a string.

Every string displayed to the user you mean.

Anyway, is this necessary for the numbers?


- Romário


---
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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: 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 235-240
> > 
> >
> > 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.
> 
> Mark Gaiser wrote:
> 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.

.. ignore my comment above :p


- 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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Romário Rios]

> On Nov. 6, 2012, 5:53 p.m., Daker Pinheiro wrote:
> > You should move the javascript functions to a separated Javascript file.
> > It would keep the plasmoid modularized.
> > A repeater could be used for creating the digits button.

>You should move the javascript functions to a separated Javascript file.
See above response to Mark.


- Romário


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


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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Daker Pinheiro]

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


You should move the javascript functions to a separated Javascript file.
It would keep the plasmoid modularized.
A repeater could be used for creating the digits button.

- Daker Pinheiro


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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Romário Rios]

---
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.


Changes
---

Corrected bug pointed by aseigo. Quicker than I thought.


Description
---

This diff replaces the C++ Calculator applet by its QMLfied version, which 
seems to be feature-par with the latter.


Diffs (updated)
-

  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

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Romário Rios]

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

(Updated Nov. 6, 2012, 2:42 a.m.)


Review request for Plasma.


Changes
---

Conformation to http://community.kde.org/Plasma/QMLStyle

(The reported bug still remains, though. Will take a look ASAP)


Description
---

This diff replaces the C++ Calculator applet by its QMLfied version, which 
seems to be feature-par with the latter.


Diffs (updated)
-

  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

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Aaron J. Seigo]

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


this needs to be modified to match http://community.kde.org/Plasma/QMLStyle

things like the opening { of a function goes on the same line: function foo() {

i also noticed a bug when trying it out -> if you enter 36*59= and then press 5 
it will show 595. it should instead start a new number sequence once = is 
pressed, just as it does with other operators. in fact, pressing a number after 
= should be the same as first pressing AC :)

great work otherwise! can't wait to see this in


applets/calculator/package/contents/ui/calculator.qml


id goes first above properties


- Aaron J. Seigo


On Nov. 1, 2012, 11:36 p.m., Romário Rios wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107001/
> ---
> 
> (Updated Nov. 1, 2012, 11:36 p.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
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Port the Calculator applet to QML

[Romário Rios]

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

(Updated Nov. 1, 2012, 11:36 p.m.)


Review request for Plasma.


Changes
---

Applet is now focused when the plasmoid pops up


Description
---

This diff replaces the C++ Calculator applet by its QMLfied version, which 
seems to be feature-par with the latter.


Diffs (updated)
-

  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 (updated)
---

Tested as both applet and popupapplet, with the Air theme and the Oxygen theme. 
It seems to work fine.


Thanks,

Romário Rios

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Port the Calculator applet to QML

[Romário Rios]

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

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 isn't keyboard-focused as a popupapplet (the user has to click it). Other 
than that, it seems to work fine.


Thanks,

Romário Rios

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel