----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124963/#review86728 -----------------------------------------------------------
src/knewpasswordwidget.cpp (line 291) <https://git.reviewboard.kde.org/r/124963/#comment59685> 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) <https://git.reviewboard.kde.org/r/124963/#comment59684> 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
