D7011: Extract lineedit password

Christoph Feck noreply at phabricator.kde.org
Mon Jul 31 15:55:27 UTC 2017


cfeck added a comment.


  Any reason it should be PasswordLineEdit, and not KPasswordLineEdit? I do not care about names, but I care about consistency.

INLINE COMMENTS

> knewpasswordwidgettest.cpp:63
>      const QString password = QStringLiteral("1234");
> -    linePassword->setText(password);
> +    linePassword->passwordLineEdit()->setText(password);
>      lineVerifyPassword->setText(password);

Does using setPassword() not work for these tests?

> knewpasswordwidget.cpp:75
>  
> -    connect(ui.linePassword, SIGNAL(textChanged(QString)), q, SLOT(_k_showToggleEchoModeAction(QString)));
> -    connect(ui.linePassword, SIGNAL(textChanged(QString)), q, SLOT(_k_textChanged()));
> +    connect(ui.linePassword->passwordLineEdit(), SIGNAL(textChanged(QString)), q, SLOT(_k_textChanged()));
>      connect(ui.lineVerifyPassword, SIGNAL(textChanged(QString)), q, SLOT(_k_textChanged()));

See discussion about properies.

> knewpasswordwidget.ui:81
>       </property>
> +     <property name="sizeHint" stdset="0">
> +      <size>

Why this sizeHint?

> kpassworddialog.ui:191
>      </hint>
> -  </hints>
> +   </hints>
>    </connection>

Revert manual white-space edit.

> passwordlineedit.h:29
> +{
> +    Q_OBJECT
> +public:

Please add a Q_PROPERTY for password, including passwordChanged() signal (NOTIFY method).

Applications should not have to deal with the passwordLineEdit() function in the regular use-case, but having the accessor function for special purpose is a good idea.

> passwordlineedit.h:46
> +     */
> +    void setPassword(const QString &p);
> +

Either remove the 'p', or name it 'password'.

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

To: mlaurent, cfeck, dfaure, elvisangelaccio
Cc: elvisangelaccio, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170731/1ac6fa49/attachment.html>


More information about the Kde-frameworks-devel mailing list