D5411: restore the "current" color scheme concept

David Edmundson noreply at phabricator.kde.org
Thu Apr 20 11:52:41 UTC 2017


davidedmundson added inline comments.

INLINE COMMENTS

> colorscm.cpp:134
> +    schemeList->insertItem(0, currentitem);
> +    schemeList->blockSignals(true); // don't emit changed signals
> +    schemeList->setCurrentItem(currentitem);

What is this about?
blocking signals is a sign of something else being seriously wrong.

> colorscm.cpp:416
>          QListWidgetItem *item = schemeList->item(i);
>          if(item->text() == i18nc("Default color scheme", "Default")) {
>              // If editing the default scheme, force a reload, else select the default scheme

Heh, there's some leftovers of the old code here.

At least it makes adding this back easier :)

> colorscm.cpp:447
> +    //Default scheme
> +    } else if (schemeList->currentRow() == 1) {
> +        KSharedConfigPtr config = m_config;

Sometime's we're comparing the name to find out which is "default" sometimes using index.

> colorscm.cpp:458-459
>      dialog->show();
>      connect(dialog, &SchemeEditorDialog::accepted, [=](){ this->populateSchemeList(); });
>      connect(dialog, &SchemeEditorDialog::rejected, [=](){ this->populateSchemeList(); });
>  }

If the current design is for saving to be handled by signaling from the dialog to the KCM which does the actual saving. You should handle apply in the same way.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D5411

To: mart, #plasma
Cc: davidedmundson, plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170420/13b92d7c/attachment.html>


More information about the Plasma-devel mailing list