D15645: [WIP] Add scheme selection menu with a "System" entry.

Kai Uwe Broulik noreply at phabricator.kde.org
Fri Sep 21 13:50:03 BST 2018


broulik added inline comments.

INLINE COMMENTS

> kcolorschememanager.cpp:203
> +        QAction *const resetAction = new QAction(index.data(Qt::DecorationRole).value<QIcon>(),
> +                                                 QStringLiteral("System (%1)").arg(systemScheme),
> +                                                 menu);

This needs translation, I also think "System" isn't a very good name for this. How about "System Default" or just "Default"?

> kcolorschememanager.cpp:214-228
> +    connect(group, &QActionGroup::triggered, [this](QAction *action) {
> +        activateScheme(action->data().toModelIndex());
> +    });
> +
> +    for (int i = 0; i < d->model->rowCount(); ++i) {
> +        QModelIndex index = d->model->index(i);
> +        QAction *action = new QAction(index.data(Qt::DecorationRole).value<QIcon>(), index.data().toString(), menu);

All of this is duplicated from`createSchemeSelectionMenu`, it should be split into a separate method so it can be reused

> kcolorschememanager.h:127
> +     */
> +    KActionMenu *createSchemeSelectionMenuWithDefault(const QIcon &icon, const QString &text, const QString &selectedSchemeName, QObject *parent);
> +

`WithDefaultEntry`?

REPOSITORY
  R265 KConfigWidgets

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

To: amhndu, #frameworks
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180921/cca4326b/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list