This revision was automatically updated to reflect the committed changes.
Closed by commit R295:6d6e2427f8e2: Update KPluginSelector to allow KCM to show
good state for reset, apply and… (authored by bport).
REPOSITORY
R295 KCMUtils
CHANGES SINCE LAST UPDATE
meven accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R295 KCMUtils
REVISION DETAIL
https://phabricator.kde.org/D26466
To: bport, #plasma, ervin, crossi, meven
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
bport marked an inline comment as done.
REPOSITORY
R295 KCMUtils
REVISION DETAIL
https://phabricator.kde.org/D26466
To: bport, #plasma, ervin, crossi, meven
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
ervin accepted this revision.
REPOSITORY
R295 KCMUtils
REVISION DETAIL
https://phabricator.kde.org/D26466
To: bport, #plasma, ervin, crossi, meven
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
bport marked 9 inline comments as done.
REPOSITORY
R295 KCMUtils
REVISION DETAIL
https://phabricator.kde.org/D26466
To: bport, #plasma, ervin, crossi, meven
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
bport updated this revision to Diff 72906.
bport added a comment.
Increment version signal was introduced and remove uneeded blank line
REPOSITORY
R295 KCMUtils
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D26466?vs=72885=72906
REVISION DETAIL
ervin added inline comments.
INLINE COMMENTS
> bport wrote in kpluginselector_p.h:205
> The method declaration changed and the connected slot already provide the
> value
Right, I missed the relevant connect.
REPOSITORY
R295 KCMUtils
REVISION DETAIL
https://phabricator.kde.org/D26466
To:
ervin added inline comments.
INLINE COMMENTS
> bport wrote in kpluginselector.cpp:389
> isPluginEnabled represent the current state (i.e. after load) so we compare
> the file value with default value
I thought that was the current GUI state not the current storage state and got
confused. I
bport added inline comments.
INLINE COMMENTS
> ervin wrote in kpluginselector.cpp:389
> This logic looks wrong to me.
>
> isChanged indicates if an entry state was changed during the course of the
> call to defaults(). It's very possible it's false when getting out of the
> loop.
> If that's
ervin requested changes to this revision.
ervin added inline comments.
INLINE COMMENTS
> kpluginselector.cpp:286
>
> +connect(this, ::changed, [=]{ emit
> defaulted(this->isDefault()); });
> +
We generally don't "this->", also you're capturing too much with =, capturing
this would be
meven requested changes to this revision.
meven added a comment.
This revision now requires changes to proceed.
Apart from @since version, seems fine to me
INLINE COMMENTS
> kpluginselector.h:234
> + * Emitted after configuration is changed, tell if configuration
> represent default or
bport added a dependent revision: D26467: KCM runners: fix default button.
REPOSITORY
R295 KCMUtils
REVISION DETAIL
https://phabricator.kde.org/D26466
To: bport, #plasma, ervin, crossi, meven
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
bport created this revision.
bport added reviewers: Plasma, ervin, crossi, meven.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
bport requested review of this revision.
REVISION SUMMARY
- emit changed with good value to ensure reset button is activated
13 matches
Mail list logo
