[Kde-pim] Review Request 110994: kmail-mobile: make NewMailPage a PlasmaComponents Page

[Michael Bohlender]

> On June 13, 2013, 1:14 p.m., Kevin Krammer wrote:
> > mobile/mail/NewMailPage.qml, line 43
> > <http://git.reviewboard.kde.org/r/110994/diff/2/?file=149822#file149822line43>
> >
> >     Is this referencing the id of the pageStack or is this a local property of PlasmaComponents.Page?
> 
> Michael Bohlender wrote:
>     This is the id of the pageStack. Should I use a more descriptive name for it like mainPageStack?
> 
> Kevin Krammer wrote:
>     Depending on an id that is defined outside the scope of the file sounds a bit brittle to me.
>     What about this: add a signal and emit it instead of calling pageStack.pop() and have the page stack react to that.
>

ups....
I checked the API instead of some code samples. turns out I it is a local property. I was confused by the fact that Page can also be used with things like PageRow and looked like it was used by id in bodega-client (something like itemBrowser.pop()). Sorry. I will remember to check the API refernce instead of assuming from code I see....
Stupid mistake. On the upside: the code seems to be fine and ready for commit ?


- Michael


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


On June 13, 2013, 12:30 p.m., Michael Bohlender wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110994/
> -----------------------------------------------------------
> 
> (Updated June 13, 2013, 12:30 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> kmail-mobile: make NewMailPage.qml a PlasmaComponents Page.
> 
> 
> Diffs
> -----
> 
>   mobile/mail/KMailActions.qml db2be7d 
>   mobile/mail/NewMailPage.qml 9d9dc06 
>   mobile/mail/StartPage.qml cb2ac84 
> 
> Diff: http://git.reviewboard.kde.org/r/110994/diff/
> 
> 
> Testing
> -------
> 
> compiled, installed, started from draft, discarded by prev. button
> 
> 
> Thanks,
> 
> Michael Bohlender
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/