Review Request: Add a new KPasswordLineEditidget

[Pino Toscano]

> On 2010-01-15 00:42:42, Michael Pyne wrote:
> > trunk/KDE/kdelibs/kdeui/widgets/kpasswordlineedit.cpp, line 38
> > <http://reviewboard.kde.org/r/2591/diff/1/?file=17070#file17070line38>
> >
> >     Another mem leak. (IIRC QLayouts do not take ownership of widgets, they only arrange them)

They don't, but widgets will be reparented to the widget of the layout.
Anyway, slightly cleaner to declare the corrent parent at construction.


> On 2010-01-15 00:42:42, Michael Pyne wrote:
> > trunk/KDE/kdelibs/kdeui/widgets/kpasswordlineedit.cpp, line 41
> > <http://reviewboard.kde.org/r/2591/diff/1/?file=17070#file17070line41>
> >
> >     I don't do color changes much but I think the "palette" property of the QWidget is the best way to change the color (and you may be able to find a suitable color role for any color theme instead of hardcoding red this way as well)

Or, even better, use KColorScheme to get the right negative color according to the current color scheme.


> On 2010-01-15 00:42:42, Michael Pyne wrote:
> > trunk/KDE/kdelibs/kdeui/widgets/kpasswordlineedit.h, line 76
> > <http://reviewboard.kde.org/r/2591/diff/1/?file=17069#file17069line76>
> >
> >     You should prefer Qt-style naming instead of Java-style.
> >     
> >     i.e. simply passwordKLineEdit(), likewise for the other getters.

Or a simplier lineEdit() :)


- Pino


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


On 2010-01-14 13:56:08, Charles Ghislain wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2591/
> -----------------------------------------------------------
> 
> (Updated 2010-01-14 13:56:08)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Hi,
> 
> I started adding a warning label to kpassworddialog, then i noticed that password fields were used at various place outside a kpassworddialog or a knewpassworddialog. Take for instance kmail, konversation,
> kdm, ... I was told the idea is that applications should use those dialogs, but the reallity seems different. Implementing the capslock warning in a new kpasswordlineedit allow all those apps to get the benefits.
> 
> This is my first contribution, I'm new to C++ and KDE and Qt in general. I tried copying from other sources i found, but there are still issues i can't resolve by myself yet :
> - Memory leaks : i don't know were is it - should i delete the childeren widgets in the destructor?
> - Formatting : i guess it refers to the widget having differenc height when the label is shown or not. I don't know how to make sure the widget keeps a fixed height even if one of his childeren is hidden. I could just set the text to label to a whitespace and keep is shown, but i guess there is a proper way.
> - Warning color : I used hardcoded red, i didn't find how to use themed colors
> - Others issues i don't think of right know.
> 
> Thanks,
> 
> Charly
> 
> 
> This addresses bug 91970.
>     https://bugs.kde.org/show_bug.cgi?id=91970
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kdeui/CMakeLists.txt 1074640 
>   trunk/KDE/kdelibs/kdeui/widgets/kpasswordlineedit.h PRE-CREATION 
>   trunk/KDE/kdelibs/kdeui/widgets/kpasswordlineedit.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/kdewidgets/kde.widgets 1074640 
> 
> Diff: http://reviewboard.kde.org/r/2591/diff
> 
> 
> Testing
> -------
> 
> I tested in designer and replaced the password field of kpassworddialog with it.
> 
> 
> Thanks,
> 
> Charles
> 
>