subject:"Review Request 124963\: New widget\: KNewPasswordWidget"



> On Oct. 11, 2015, 9:16 p.m., David Faure wrote:
> > src/knewpasswordwidget.cpp, line 146
> > 
> >
> > (This method is really a candidate for being unittested directly; just 
> > a thought)

This is not a public method, though. Would you prefer to make it such?


- Elvis


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86678
---


On Oct. 12, 2015, 4:13 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Oct. 12, 2015, 4:13 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/
---

(Updated Oct. 12, 2015, 4:13 p.m.)

Review request for KDE Frameworks and Christoph Feck.

Changes
---

I'm not sure about the call to `update()`. David, could you have another look
before I land?

Repository: kwidgetsaddons

Description
---

This widget is a stripped-down version of KNewPasswordDialog, without any
dialog-specific stuff.

### Why a new widget?

### Features

### Use cases
At least the following:

* Ark (new archive dialog)
* KNewPasswordDialog refactoring (my next RR if this one gets accepted)

Diffs (updated)
-

Diff: https://git.reviewboard.kde.org/r/124963/diff/

Testing
---

File Attachments

knewpasswordwidget1.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
knewpasswordwidget2.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
knewpasswordwidget3.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png

Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

2015-10-12 Thread David Faure


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86728
---



src/knewpasswordwidget.cpp (line 291)


Looks like I gave slightly incorrect advice, this should be qBound(min, 
value, max)
  -> please revise all qBound usage to ensure they are correct. And you 
could add unit tests for these methods, to test the out of bounds behavior - 
they would have caught this bug :-)



src/knewpasswordwidget.cpp (line 303)


The if() is not needed.

I was just explaining what update is useful for.
Just call update() when changing a setting that affects appearance. Qt will 
then "do the right thing" :-)


- David Faure


On Oct. 12, 2015, 4:13 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Oct. 12, 2015, 4:13 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/
---

(Updated Oct. 12, 2015, 5:01 p.m.)

Review request for KDE Frameworks and Christoph Feck.

Changes
---

Fixed qBounds + tests. Removed unnecessary isVisible() call.

Repository: kwidgetsaddons

Description
---

This widget is a stripped-down version of KNewPasswordDialog, without any
dialog-specific stuff.

### Why a new widget?

### Features

### Use cases
At least the following:

* Ark (new archive dialog)
* KNewPasswordDialog refactoring (my next RR if this one gets accepted)

Diffs (updated)
-

Diff: https://git.reviewboard.kde.org/r/124963/diff/

Testing
---

File Attachments

knewpasswordwidget1.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
knewpasswordwidget2.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
knewpasswordwidget3.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png

Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/
---

(Updated Oct. 12, 2015, 8:22 p.m.)

Review request for KDE Frameworks and Christoph Feck.

Changes
---

No hurry to ship :)
I updated the tests with the out-of-bound checks, and also the doxygen.

Repository: kwidgetsaddons

Description
---

This widget is a stripped-down version of KNewPasswordDialog, without any
dialog-specific stuff.

### Why a new widget?

### Features

### Use cases
At least the following:

* Ark (new archive dialog)
* KNewPasswordDialog refactoring (my next RR if this one gets accepted)

Diffs (updated)
-

Diff: https://git.reviewboard.kde.org/r/124963/diff/

Testing
---

File Attachments

knewpasswordwidget1.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
knewpasswordwidget2.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
knewpasswordwidget3.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png

Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

2015-10-12 Thread David Faure


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86742
---

Ship it!


I have suggestions on how to improve the unit test, but that's a never ending 
game, so feel free to push :-)


autotests/knewpasswordwidgettest.cpp (line 135)


typo: Reasonoble -> Reasonable



autotests/knewpasswordwidgettest.cpp (line 141)


More precisely, QCOMPARE(pwdWidget.reasonablePasswordLength(), 10);

Testing out of bounds behaviour would mean testing what happens after 
setReasonablePasswordLength(0)
and
setReasonablePasswordLength(100) (or whatever is out of bounds)


- David Faure


On Oct. 12, 2015, 5:01 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Oct. 12, 2015, 5:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

2015-10-12 Thread David Faure


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86774
---

Ship it!



autotests/knewpasswordwidgettest.cpp (line 143)


So if set...(0) would set the value to 95, that would be fine for you? :)

I would QCOMPARE(, 1), if that's what we expect to happen.
(same reasoning for the 4 QVERIFY statements)


- David Faure


On Oct. 12, 2015, 8:22 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Oct. 12, 2015, 8:22 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

2015-10-11 Thread David Faure


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86678
---

Ship it!



src/knewpasswordwidget.cpp (line 97)


this doesn't have to be done every time; do it in the constructor (or in 
the .ui file)



src/knewpasswordwidget.cpp (line 101)


qBound(pwstrength, 0, 100) :-)



src/knewpasswordwidget.cpp (line 146)


(This method is really a candidate for being unittested directly; just a 
thought)



src/knewpasswordwidget.cpp (line 162)


use leftRef() instead of left() to avoid N string copies here.



src/knewpasswordwidget.cpp (line 307)


+ call update(), in case the widget is already shown?


- David Faure


On Oct. 10, 2015, 3:50 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Oct. 10, 2015, 3:50 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

2015-10-11 Thread Kai Uwe Broulik



> On Okt. 11, 2015, 9:16 nachm., David Faure wrote:
> > src/knewpasswordwidget.cpp, line 101
> > 
> >
> > qBound(pwstrength, 0, 100) :-)

qBound(0, pwstrength, 100) actually


- Kai Uwe


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86678
---


On Okt. 10, 2015, 3:50 nachm., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Okt. 10, 2015, 3:50 nachm.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget



> On Oct. 10, 2015, 3:22 p.m., David Faure wrote:
> > src/knewpasswordwidget.cpp, line 299
> > 
> >
> > No, qBound would set it to maximumPasswordLength(), which acts as a 
> > maximum value.

I figured why I was confused: there is a bug in the documentation of `qBound`:

*Returns value bounded by min and max. This is equivalent to qMax(min, 
qMin(value, max)).*

should instead be:

*Returns value bounded by min and max. This is equivalent to qMax(min, 
qMin(**MAX**, **VALUE**)).*


- Elvis


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86640
---


On Aug. 30, 2015, 8:26 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Aug. 30, 2015, 8:26 a.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget



> On Oct. 10, 2015, 1:56 p.m., David Faure wrote:
> > src/knewpasswordwidget.h, line 65
> > 
> >
> > these 9 lines could be done in one or two:
> > 
> > const bool passOk = m_passwordWidget->passwordStatus() == 
> > WeakPassword || m_passwordWidget->passwordStatus() == StrongPassword;
> > m_buttonBox->button(QDialogButtonBox::Ok)->setEnabled(passOk);
> > 
> > But I guess you wanted to show a switch so people could add more code 
> > in there like showing "your password is too weak"?

Yeah that was the idea, but indeed is similar to what we already show in the 
code snippet right below this one. I'm ok with using your version if you want.


> On Oct. 10, 2015, 1:56 p.m., David Faure wrote:
> > src/knewpasswordwidget.cpp, line 299
> > 
> >
> > the whole method in a single line:
> > 
> > d->reasonablePasswordLength = qBound(reasonableLength, 1, 
> > maximumPasswordLength());

Are you sure this is equivalent? If I use `qBound` and `reasonableLenght >= 
maximumPasswordLength()`, then `d->reasonablePasswordLength` would be set to 
`reasonableLenght`, while the old algorithm would set it to 
`maximumPasswordLength()`. Or am I missing something?


- Elvis


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86631
---


On Aug. 30, 2015, 8:26 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Aug. 30, 2015, 8:26 a.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget



> On Oct. 10, 2015, 1:56 p.m., David Faure wrote:
> > src/knewpasswordwidget.h, line 65
> > 
> >
> > these 9 lines could be done in one or two:
> > 
> > const bool passOk = m_passwordWidget->passwordStatus() == 
> > WeakPassword || m_passwordWidget->passwordStatus() == StrongPassword;
> > m_buttonBox->button(QDialogButtonBox::Ok)->setEnabled(passOk);
> > 
> > But I guess you wanted to show a switch so people could add more code 
> > in there like showing "your password is too weak"?
> 
> Elvis Angelaccio wrote:
> Yeah that was the idea, but indeed is similar to what we already show in 
> the code snippet right below this one. I'm ok with using your version if you 
> want.
> 
> David Faure wrote:
> I'll let you decide, based on what a real application would most probably 
> need.

Well, if this would be a good place to show "your password is too weak", then 
at least add a comment about that in the sample code, inside the switch - and 
if not, then use my version.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86631
---


On Aug. 30, 2015, 8:26 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Aug. 30, 2015, 8:26 a.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget



> On Oct. 10, 2015, 3:22 p.m., David Faure wrote:
> > src/knewpasswordwidget.cpp, line 299
> > 
> >
> > No, qBound would set it to maximumPasswordLength(), which acts as a 
> > maximum value.
> 
> Elvis Angelaccio wrote:
> I figured why I was confused: there is a bug in the documentation of 
> `qBound`:
> 
> *Returns value bounded by min and max. This is equivalent to qMax(min, 
> qMin(value, max)).*
> 
> should instead be:
> 
> *Returns value bounded by min and max. This is equivalent to qMax(min, 
> qMin(**MAX**, **VALUE**)).*

You're still confused, because there is no difference between the result of 
qMin(A, B) and qMin(B, A) ... it returns the smallest value of the two :-)
=> no bug in the docu.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86640
---


On Aug. 30, 2015, 8:26 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Aug. 30, 2015, 8:26 a.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86631
---



autotests/knewpasswordwidgettest.cpp (line 23)


this includes all of QtTest and all of QtCore.
Prefer #include  (+ #include  if using that).



autotests/knewpasswordwidgettest.cpp (line 60)


if you used QString instead of auto here, the QLatin1String->QString 
conversion would only happen once...



autotests/knewpasswordwidgettest.cpp (line 128)


Keep Ass ? :-)



src/knewpasswordwidget.h (line 65)


these 9 lines could be done in one or two:

const bool passOk = m_passwordWidget->passwordStatus() == WeakPassword 
|| m_passwordWidget->passwordStatus() == StrongPassword;
m_buttonBox->button(QDialogButtonBox::Ok)->setEnabled(passOk);

But I guess you wanted to show a switch so people could add more code in 
there like showing "your password is too weak"?



src/knewpasswordwidget.h (line 100)


missing @since 5.16 ?



src/knewpasswordwidget.h (line 122)


typo: lenght -> length



src/knewpasswordwidget.cpp (line 299)


the whole method in a single line:

d->reasonablePasswordLength = qBound(reasonableLength, 1, 
maximumPasswordLength());



src/knewpasswordwidget.cpp (line 311)


same here, use qBound



src/knewpasswordwidget.cpp (line 317)


Ouch! A stylesheet! I strongly recommend against that. It messe up fonts 
and palettes.

Use pal = palette(), pal.setColor(Base, ...), setPalette(pal) and 
setAutoFillBackground(true) on the lineedit whose background color you want to 
change.


- David Faure


On Aug. 30, 2015, 8:26 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Aug. 30, 2015, 8:26 a.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86640
---



src/knewpasswordwidget.cpp (line 299)


No, qBound would set it to maximumPasswordLength(), which acts as a maximum 
value.


- David Faure


On Aug. 30, 2015, 8:26 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Aug. 30, 2015, 8:26 a.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget



> On Oct. 10, 2015, 1:56 p.m., David Faure wrote:
> > src/knewpasswordwidget.h, line 65
> > 
> >
> > these 9 lines could be done in one or two:
> > 
> > const bool passOk = m_passwordWidget->passwordStatus() == 
> > WeakPassword || m_passwordWidget->passwordStatus() == StrongPassword;
> > m_buttonBox->button(QDialogButtonBox::Ok)->setEnabled(passOk);
> > 
> > But I guess you wanted to show a switch so people could add more code 
> > in there like showing "your password is too weak"?
> 
> Elvis Angelaccio wrote:
> Yeah that was the idea, but indeed is similar to what we already show in 
> the code snippet right below this one. I'm ok with using your version if you 
> want.

I'll let you decide, based on what a real application would most probably need.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86631
---


On Aug. 30, 2015, 8:26 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Aug. 30, 2015, 8:26 a.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/
---

(Updated Oct. 10, 2015, 3:50 p.m.)

Review request for KDE Frameworks and Christoph Feck.

Changes
---

Incorporate David's suggestions. I had to add a new QColor member to store the
default background color, while before I would use an empty string as
stylesheet.

Repository: kwidgetsaddons

Description
---

This widget is a stripped-down version of KNewPasswordDialog, without any
dialog-specific stuff.

### Why a new widget?

### Features

### Use cases
At least the following:

* Ark (new archive dialog)
* KNewPasswordDialog refactoring (my next RR if this one gets accepted)

Diffs (updated)
-

Diff: https://git.reviewboard.kde.org/r/124963/diff/

Testing
---

File Attachments

knewpasswordwidget1.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
knewpasswordwidget2.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
knewpasswordwidget3.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png

Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget



> On Oct. 10, 2015, 3:22 p.m., David Faure wrote:
> > src/knewpasswordwidget.cpp, line 299
> > 
> >
> > No, qBound would set it to maximumPasswordLength(), which acts as a 
> > maximum value.
> 
> Elvis Angelaccio wrote:
> I figured why I was confused: there is a bug in the documentation of 
> `qBound`:
> 
> *Returns value bounded by min and max. This is equivalent to qMax(min, 
> qMin(value, max)).*
> 
> should instead be:
> 
> *Returns value bounded by min and max. This is equivalent to qMax(min, 
> qMin(**MAX**, **VALUE**)).*
> 
> David Faure wrote:
> You're still confused, because there is no difference between the result 
> of qMin(A, B) and qMin(B, A) ... it returns the smallest value of the two :-)
> => no bug in the docu.

Of course there isn't. Sorry for the noise :)


- Elvis


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86640
---


On Aug. 30, 2015, 8:26 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Aug. 30, 2015, 8:26 a.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

2015-10-07 Thread Elvis Angelaccio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review86458
---


Ping?

- Elvis Angelaccio


On Aug. 30, 2015, 8:26 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124963/
> ---
> 
> (Updated Aug. 30, 2015, 8:26 a.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> This widget is a stripped-down version of KNewPasswordDialog, without any 
> dialog-specific stuff.
> 
> ### Why a new widget?
> 
> This widget is meant to be easily embedded in any custom password dialog, 
> including KNewPasswordDialog. It's the least common denominator of features 
> that any password dialog should offer to the user.
> 
> ### Features
> 
> * Password visibility action (same as RR 124698). The password verification 
> field is hidden if the user shows the password.
> * Background warning colour. The password verification field gets a coloured 
> background whenever the password and its verification don't match. (feature 
> borrowed from Keepass)
> * Password status signal. This allows the upper level dialogs to update their 
> stuff (enable/disable OK button, show warnings for low password strength, 
> etc.)
> * Password strength bar can be hidden.
> * Unit test.
> 
> ### Use cases
> At least the following:
> 
> * Ark (new archive dialog)
> * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
>   autotests/knewpasswordwidgettest.h PRE-CREATION 
>   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
>   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
>   src/knewpasswordwidget.h PRE-CREATION 
>   src/knewpasswordwidget.cpp PRE-CREATION 
>   src/knewpasswordwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124963/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> knewpasswordwidget1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
> knewpasswordwidget2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
> knewpasswordwidget3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

2015-08-30 Thread Elvis Angelaccio

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/
---

(Updated Ago. 30, 2015, 8:26 a.m.)

Review request for KDE Frameworks and Christoph Feck.

Changes
---

Reverted switch to i18n. I placed the context as second argument of tr(). I
hope it's ok.

Repository: kwidgetsaddons

Description
---

This widget is a stripped-down version of KNewPasswordDialog, without any
dialog-specific stuff.

### Why a new widget?

### Features

### Use cases
At least the following:

* Ark (new archive dialog)
* KNewPasswordDialog refactoring (my next RR if this one gets accepted)

Diffs (updated)
-

Diff: https://git.reviewboard.kde.org/r/124963/diff/

Testing
---

File Attachments

knewpasswordwidget1.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
knewpasswordwidget2.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
knewpasswordwidget3.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png

Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

2015-08-29 Thread Sune Vuorela



 On Aug. 28, 2015, 11:30 a.m., Lamarque Souza wrote:
  src/knewpasswordwidget.cpp, line 75
  https://git.reviewboard.kde.org/r/124963/diff/1/?file=399453#file399453line75
 
  i18n instead of tr

Aren't we in a tier1 framework where i18n and friends aren't available?


- Sune


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review84525
---


On Aug. 28, 2015, 3:52 p.m., Elvis Angelaccio wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124963/
 ---
 
 (Updated Aug. 28, 2015, 3:52 p.m.)
 
 
 Review request for KDE Frameworks and Christoph Feck.
 
 
 Repository: kwidgetsaddons
 
 
 Description
 ---
 
 This widget is a stripped-down version of KNewPasswordDialog, without any 
 dialog-specific stuff.
 
 ### Why a new widget?
 
 This widget is meant to be easily embedded in any custom password dialog, 
 including KNewPasswordDialog. It's the least common denominator of features 
 that any password dialog should offer to the user.
 
 ### Features
 
 * Password visibility action (same as RR 124698). The password verification 
 field is hidden if the user shows the password.
 * Background warning colour. The password verification field gets a coloured 
 background whenever the password and its verification don't match. (feature 
 borrowed from Keepass)
 * Password status signal. This allows the upper level dialogs to update their 
 stuff (enable/disable OK button, show warnings for low password strength, 
 etc.)
 * Password strength bar can be hidden.
 * Unit test.
 
 ### Use cases
 At least the following:
 
 * Ark (new archive dialog)
 * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
 
 
 Diffs
 -
 
   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
   autotests/knewpasswordwidgettest.h PRE-CREATION 
   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
   src/knewpasswordwidget.h PRE-CREATION 
   src/knewpasswordwidget.cpp PRE-CREATION 
   src/knewpasswordwidget.ui PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/124963/diff/
 
 
 Testing
 ---
 
 
 File Attachments
 
 
 knewpasswordwidget1.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
 knewpasswordwidget2.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
 knewpasswordwidget3.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
 
 
 Thanks,
 
 Elvis Angelaccio
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

2015-08-29 Thread Lamarque Souza



 On Aug. 28, 2015, 11:30 a.m., Lamarque Souza wrote:
  src/knewpasswordwidget.cpp, line 75
  https://git.reviewboard.kde.org/r/124963/diff/1/?file=399453#file399453line75
 
  i18n instead of tr
 
 Sune Vuorela wrote:
 Aren't we in a tier1 framework where i18n and friends aren't available?

Yeah, you're right. Please revert the i18n change and sorry for not noticing 
that before.


- Lamarque


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review84525
---


On Aug. 28, 2015, 3:52 p.m., Elvis Angelaccio wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124963/
 ---
 
 (Updated Aug. 28, 2015, 3:52 p.m.)
 
 
 Review request for KDE Frameworks and Christoph Feck.
 
 
 Repository: kwidgetsaddons
 
 
 Description
 ---
 
 This widget is a stripped-down version of KNewPasswordDialog, without any 
 dialog-specific stuff.
 
 ### Why a new widget?
 
 This widget is meant to be easily embedded in any custom password dialog, 
 including KNewPasswordDialog. It's the least common denominator of features 
 that any password dialog should offer to the user.
 
 ### Features
 
 * Password visibility action (same as RR 124698). The password verification 
 field is hidden if the user shows the password.
 * Background warning colour. The password verification field gets a coloured 
 background whenever the password and its verification don't match. (feature 
 borrowed from Keepass)
 * Password status signal. This allows the upper level dialogs to update their 
 stuff (enable/disable OK button, show warnings for low password strength, 
 etc.)
 * Password strength bar can be hidden.
 * Unit test.
 
 ### Use cases
 At least the following:
 
 * Ark (new archive dialog)
 * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
 
 
 Diffs
 -
 
   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
   autotests/knewpasswordwidgettest.h PRE-CREATION 
   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
   src/knewpasswordwidget.h PRE-CREATION 
   src/knewpasswordwidget.cpp PRE-CREATION 
   src/knewpasswordwidget.ui PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/124963/diff/
 
 
 Testing
 ---
 
 
 File Attachments
 
 
 knewpasswordwidget1.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
 knewpasswordwidget2.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
 knewpasswordwidget3.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
 
 
 Thanks,
 
 Elvis Angelaccio
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

2015-08-29 Thread Sune Vuorela



 On Aug. 28, 2015, 11:30 a.m., Lamarque Souza wrote:
  src/knewpasswordwidget.cpp, line 75
  https://git.reviewboard.kde.org/r/124963/diff/1/?file=399453#file399453line75
 
  i18n instead of tr
 
 Sune Vuorela wrote:
 Aren't we in a tier1 framework where i18n and friends aren't available?
 
 Lamarque Souza wrote:
 Yeah, you're right. Please revert the i18n change and sorry for not 
 noticing that before.
 
 Martin Klapetek wrote:
 Fwiw, kwidgetsaddons has i18n usage already.

Not in 77e030112909e218aa85f851b289d298dc68a9f2 at least. There is some 
examples that promotes usage of i18n, but none of the actual code uses it.


- Sune


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review84525
---


On Aug. 28, 2015, 3:52 p.m., Elvis Angelaccio wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124963/
 ---
 
 (Updated Aug. 28, 2015, 3:52 p.m.)
 
 
 Review request for KDE Frameworks and Christoph Feck.
 
 
 Repository: kwidgetsaddons
 
 
 Description
 ---
 
 This widget is a stripped-down version of KNewPasswordDialog, without any 
 dialog-specific stuff.
 
 ### Why a new widget?
 
 This widget is meant to be easily embedded in any custom password dialog, 
 including KNewPasswordDialog. It's the least common denominator of features 
 that any password dialog should offer to the user.
 
 ### Features
 
 * Password visibility action (same as RR 124698). The password verification 
 field is hidden if the user shows the password.
 * Background warning colour. The password verification field gets a coloured 
 background whenever the password and its verification don't match. (feature 
 borrowed from Keepass)
 * Password status signal. This allows the upper level dialogs to update their 
 stuff (enable/disable OK button, show warnings for low password strength, 
 etc.)
 * Password strength bar can be hidden.
 * Unit test.
 
 ### Use cases
 At least the following:
 
 * Ark (new archive dialog)
 * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
 
 
 Diffs
 -
 
   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
   autotests/knewpasswordwidgettest.h PRE-CREATION 
   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
   src/knewpasswordwidget.h PRE-CREATION 
   src/knewpasswordwidget.cpp PRE-CREATION 
   src/knewpasswordwidget.ui PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/124963/diff/
 
 
 Testing
 ---
 
 
 File Attachments
 
 
 knewpasswordwidget1.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
 knewpasswordwidget2.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
 knewpasswordwidget3.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
 
 
 Thanks,
 
 Elvis Angelaccio
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 124963: New widget: KNewPasswordWidget

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/
---

Review request for KDE Frameworks and Christoph Feck.

Repository: kwidgetsaddons

Description
---

This widget is a stripped-down version of KNewPasswordDialog, without any
dialog-specific stuff.

### Why a new widget?

### Features

### Use cases
At least the following:

* Ark (new archive dialog)
* KNewPasswordDialog refactoring (my next RR if this one gets accepted)

Diffs
-

Diff: https://git.reviewboard.kde.org/r/124963/diff/

Testing
---

File Attachments

knewpasswordwidget1.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
knewpasswordwidget2.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
knewpasswordwidget3.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png

Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

2015-08-28 Thread Lamarque Souza


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review84525
---



src/knewpasswordwidget.h (line 179)
https://git.reviewboard.kde.org/r/124963/#comment58492

s/strenght/strength/



src/knewpasswordwidget.h (line 185)
https://git.reviewboard.kde.org/r/124963/#comment58493

s/has/returns/



src/knewpasswordwidget.cpp (line 35)
https://git.reviewboard.kde.org/r/124963/#comment58494

Also initialize passwordStatus and toggleEchoModeAction variables here.



src/knewpasswordwidget.cpp (line 63)
https://git.reviewboard.kde.org/r/124963/#comment58495

i18n instead of tr. You can also use ritch text mode here:

http://doc.qt.io/qt-5.4/richtext-html-subset.html



src/knewpasswordwidget.cpp (line 75)
https://git.reviewboard.kde.org/r/124963/#comment58496

i18n instead of tr



src/knewpasswordwidget.cpp (line 92)
https://git.reviewboard.kde.org/r/124963/#comment58497

Is there any documentation explaining this calculation?



src/knewpasswordwidget.cpp (line 124)
https://git.reviewboard.kde.org/r/124963/#comment58498

Code style: join this line with the next one.



src/knewpasswordwidget.cpp (line 154)
https://git.reviewboard.kde.org/r/124963/#comment58499

Please write a short explanation of what is happening here. By what I could 
understand you are calculating the number of characters with different 
categories in the password, am I correct?



src/knewpasswordwidget.cpp (line 202)
https://git.reviewboard.kde.org/r/124963/#comment58500

Before emitting this signal check if parameter status is different from 
passwordStatus. Do not emit the signal if they are equal.



src/knewpasswordwidget.cpp (line 275)
https://git.reviewboard.kde.org/r/124963/#comment58503

If maxLength is less than minimumPasswordLength the user will not be able 
to type a valid password. Should not you check for that here and use 
minimumPasswordLength instead of maxLength in that particular case?


- Lamarque Souza


On Aug. 28, 2015, 9:24 a.m., Elvis Angelaccio wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124963/
 ---
 
 (Updated Aug. 28, 2015, 9:24 a.m.)
 
 
 Review request for KDE Frameworks and Christoph Feck.
 
 
 Repository: kwidgetsaddons
 
 
 Description
 ---
 
 This widget is a stripped-down version of KNewPasswordDialog, without any 
 dialog-specific stuff.
 
 ### Why a new widget?
 
 This widget is meant to be easily embedded in any custom password dialog, 
 including KNewPasswordDialog. It's the least common denominator of features 
 that any password dialog should offer to the user.
 
 ### Features
 
 * Password visibility action (same as RR 124698). The password verification 
 field is hidden if the user shows the password.
 * Background warning colour. The password verification field gets a coloured 
 background whenever the password and its verification don't match. (feature 
 borrowed from Keepass)
 * Password status signal. This allows the upper level dialogs to update their 
 stuff (enable/disable OK button, show warnings for low password strength, 
 etc.)
 * Password strength bar can be hidden.
 * Unit test.
 
 ### Use cases
 At least the following:
 
 * Ark (new archive dialog)
 * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
 
 
 Diffs
 -
 
   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
   autotests/knewpasswordwidgettest.h PRE-CREATION 
   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
   src/knewpasswordwidget.h PRE-CREATION 
   src/knewpasswordwidget.cpp PRE-CREATION 
   src/knewpasswordwidget.ui PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/124963/diff/
 
 
 Testing
 ---
 
 
 File Attachments
 
 
 knewpasswordwidget1.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
 knewpasswordwidget2.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
 knewpasswordwidget3.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
 
 
 Thanks,
 
 Elvis Angelaccio
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/
---

(Updated Ago. 28, 2015, 1:56 p.m.)

Review request for KDE Frameworks and Christoph Feck.

Changes
---

* Fixed most of the Lamarque's issues

Repository: kwidgetsaddons

Description
---

This widget is a stripped-down version of KNewPasswordDialog, without any
dialog-specific stuff.

### Why a new widget?

### Features

### Use cases
At least the following:

* Ark (new archive dialog)
* KNewPasswordDialog refactoring (my next RR if this one gets accepted)

Diffs (updated)
-

Diff: https://git.reviewboard.kde.org/r/124963/diff/

Testing
---

File Attachments

knewpasswordwidget1.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
knewpasswordwidget2.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
knewpasswordwidget3.png

https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png

Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget



 On Ago. 28, 2015, 11:30 a.m., Lamarque Souza wrote:
  src/knewpasswordwidget.cpp, line 92
  https://git.reviewboard.kde.org/r/124963/diff/1/?file=399453#file399453line92
 
  Is there any documentation explaining this calculation?

I don't know, sorry. The whole strength calculator code is just taken from 
KNewPasswordDialog. I don't even know its original author (git blame does not 
help here).


 On Ago. 28, 2015, 11:30 a.m., Lamarque Souza wrote:
  src/knewpasswordwidget.cpp, line 154
  https://git.reviewboard.kde.org/r/124963/diff/1/?file=399453#file399453line154
 
  Please write a short explanation of what is happening here. By what I 
  could understand you are calculating the number of characters with 
  different categories in the password, am I correct?

As above, not my code :(
If this is a blocker, I can look for a different and documented algorithm and 
replace the current one.


- Elvis


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124963/#review84525
---


On Ago. 28, 2015, 1:56 p.m., Elvis Angelaccio wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124963/
 ---
 
 (Updated Ago. 28, 2015, 1:56 p.m.)
 
 
 Review request for KDE Frameworks and Christoph Feck.
 
 
 Repository: kwidgetsaddons
 
 
 Description
 ---
 
 This widget is a stripped-down version of KNewPasswordDialog, without any 
 dialog-specific stuff.
 
 ### Why a new widget?
 
 This widget is meant to be easily embedded in any custom password dialog, 
 including KNewPasswordDialog. It's the least common denominator of features 
 that any password dialog should offer to the user.
 
 ### Features
 
 * Password visibility action (same as RR 124698). The password verification 
 field is hidden if the user shows the password.
 * Background warning colour. The password verification field gets a coloured 
 background whenever the password and its verification don't match. (feature 
 borrowed from Keepass)
 * Password status signal. This allows the upper level dialogs to update their 
 stuff (enable/disable OK button, show warnings for low password strength, 
 etc.)
 * Password strength bar can be hidden.
 * Unit test.
 
 ### Use cases
 At least the following:
 
 * Ark (new archive dialog)
 * KNewPasswordDialog refactoring (my next RR if this one gets accepted)
 
 
 Diffs
 -
 
   autotests/CMakeLists.txt ac708ef33e3be89db85d43911f96e536c52f741d 
   autotests/knewpasswordwidgettest.h PRE-CREATION 
   autotests/knewpasswordwidgettest.cpp PRE-CREATION 
   src/CMakeLists.txt e03e9bbd6d73811873b0a465f86da269f4295138 
   src/knewpasswordwidget.h PRE-CREATION 
   src/knewpasswordwidget.cpp PRE-CREATION 
   src/knewpasswordwidget.ui PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/124963/diff/
 
 
 Testing
 ---
 
 
 File Attachments
 
 
 knewpasswordwidget1.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/5796469c-28b7-4151-b9cb-6a327631db75__knewpasswordwidget1.png
 knewpasswordwidget2.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/b3bfc9ac-ab8e-404c-8091-b5ad9e3e054f__knewpasswordwidget2.png
 knewpasswordwidget3.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/08/28/dfaeea4e-65da-4961-b266-986a21f54ca7__knewpasswordwidget3.png
 
 
 Thanks,
 
 Elvis Angelaccio
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124963: New widget: KNewPasswordWidget