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