ngraham added a comment.

  Very nice work. This is close to a "shipit" already IMO from my perspective. 
I just have a few more comments:

INLINE COMMENTS

> GeneralTab.qml:26
> +import org.kde.kquickcontrols 2.0 as KQuickControls
> +import org.kde.kquickcontrolsaddons 2.0 as KQuickControls
>  

You're importing two different things with the same name

> ActivitiesView.qml:66
> +                        icon.name: "configure"
> +                        tooltip: i18nc("@info:tooltip", "Configure...")
> +                        onTriggered: 
> ActivitySettings.configureActivity(model.id);

Maybe "Configure <activity name> activity..."?

> ActivitiesView.qml:73
> +                        icon.name: "edit-delete"
> +                        tooltip: i18nc("@info:tooltip", "Delete")
> +                        onTriggered: 
> ActivitySettings.deleteActivity(model.id);

Maybe "Delete <activity name> activity"?

> ActivitiesView.qml:84
> +        visible: ActivitySettings.newActivityAuthorized
> +        text: i18nd("kcm_activities5", "Create...")
> +        icon.name: "list-add"

How about "Create New..." just to emphasize that a new one is being made

> BlacklistApplicationView.qml:61
> +                width: parent.width
> +                source: "dialog-cancel"
> +                opacity: (1 - icon.opacity) * 2

`emblem-unavailable` looks nicer and seems more semantically correct. If we use 
it, it should be located in the bottom-right corner of the icon to be 
consistent with other emblem placement.

REPOSITORY
  R119 Plasma Desktop

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

To: GB_2, #plasma, #vdg, ivan
Cc: ngraham, #vdg, plasma-devel, #plasma, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

Reply via email to