D7011: Extract KPasswordLineEdit class

Christoph Feck noreply at phabricator.kde.org
Wed Aug 2 19:42:22 UTC 2017


cfeck added inline comments.

INLINE COMMENTS

> CMakeLists.txt:21
>    ksplittercollapserbuttontest.cpp
> +  kpasswordlineedittest.cpp
>    LINK_LIBRARIES Qt5::Test KF5::WidgetsAddons

If that file is sorted (at least a bit), try to insert instead of append. Same for other places.

> knewpasswordwidget.cpp:118
>  {
> -    if (ui.linePassword->echoMode() == QLineEdit::Password) {
> -        ui.linePassword->setEchoMode(QLineEdit::Normal);
> +    if (ui.linePassword->lineEdit()->echoMode() == QLineEdit::Normal) {
>          ui.lineVerifyPassword->hide();

Can you double-check this change? To me, the echoMode() checks are now reversed compared to the original code.

I have not tested the code yet :)

> kossebau wrote in kpasswordlineedit.cpp:30
> Make this a normal class KPasswordLineEditPrivate here, following fixes proposed above for the header.

Does this class need to be a QObject? If not, remove the QObject stuff.

> kpasswordlineedit.h:133
> +    void echoModeChanged(QLineEdit::EchoMode echoMode);
> +    void passwordChanged();
> +    void textChanged(const QString &text);

Also needs the const QString &password argument.

Or if I misunderstand it, what is the difference between passwordChanged() and textChanged()? This class has no text() property, so having a textChanged() looks unexpected.

REVISION DETAIL
  https://phabricator.kde.org/D7011

To: mlaurent, cfeck, dfaure, elvisangelaccio
Cc: kossebau, elvisangelaccio, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170802/32db4e53/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list