[Kde-pim] Re: Review Request: Fix the Apply button behaviour in the Configure Filters dialog (Bug 123548)
Andras Mantia
amantia at kde.org
Wed Dec 8 21:29:13 GMT 2010
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6061/#review9178
-----------------------------------------------------------
Ship it!
Looks good to me, unless somebody else has an objection and you tested it (I cannot test before the weekend), you can commit. If you don't have commit access, I can do it on the weekend.
There is only one small comment, see below.
/trunk/KDE/kdepim/kmail/kmfilterdlg.cpp
<http://svn.reviewboard.kde.org/r/6061/#comment10023>
This will call slotDialogUpdated twice, but well, it can stay, as indeed insert does both actions.
- Andras
On 2010-12-08 09:01:57, George Metaxas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6061/
> -----------------------------------------------------------
>
> (Updated 2010-12-08 09:01:57)
>
>
> Review request for KDE PIM.
>
>
> Summary
> -------
>
> Currently, the Apply button in the Configure Dialog is always enabled. This is because the button was simply added to the dialog and left unconnected to the dialog's logic. This patch implements the behaviour of the Apply button, by enabling it when a value in the dialog has changed. Apart from connecting an appropriate signal of each widget in the dialog to the Apply button logic, some signal/slot implementation was required in various classes of the MailCommon namespace, to enable the notification of a value change in some of the provided widgets (e.g. SoundTestWidget, SearchPatternEdit) or classes (e.g. FilterAction and subclasses).
>
>
> This addresses bug 123548.
> https://bugs.kde.org/show_bug.cgi?id=123548
>
>
> Diffs
> -----
>
> /trunk/KDE/kdepim/kmail/kmfilterdlg.h 1204467
> /trunk/KDE/kdepim/kmail/kmfilterdlg.cpp 1204467
> /trunk/KDE/kdepim/mailcommon/filteraction.h 1204467
> /trunk/KDE/kdepim/mailcommon/filteraction.cpp 1204467
> /trunk/KDE/kdepim/mailcommon/filteractionwidget.h 1204467
> /trunk/KDE/kdepim/mailcommon/filteractionwidget.cpp 1204467
> /trunk/KDE/kdepim/mailcommon/searchpatternedit.h 1204467
> /trunk/KDE/kdepim/mailcommon/searchpatternedit.cpp 1204467
> /trunk/KDE/kdepim/mailcommon/soundtestwidget.h 1204467
> /trunk/KDE/kdepim/mailcommon/soundtestwidget.cpp 1204467
>
> Diff: http://svn.reviewboard.kde.org/r/6061/diff
>
>
> Testing
> -------
>
> Tested the dialog by adding/removing/changing filters, altering filter order, adding/removing/altering filter actions, adding/removing/altering filter criteria.
>
>
> Thanks,
>
> George
>
>
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/
More information about the kde-pim
mailing list