ngraham added a comment.
  Nice!
  
  I notice that for some reason, the slider's last two values now display 81 
and 162 in the text field.
  
  Also while you're at it, could you make the icon grid alphabetically ordered? 
Right now I can't tell the sort order and it just looks random.

INLINE COMMENTS

> Preview.qml:83
>              Layout.fillWidth: true
> -            onClicked: clipboard(preview.iconName);
> -        }
> -        PlasmaComponents.ToolButton {
> -            text: pickerMode ?  i18n("Insert QtQuick code") : i18n("Copy 
> QtQuick code to clipboard")
> -            iconSource: "edit-copy"
> -            Layout.fillWidth: true
> -            onClicked: {
> -                var code = "/* Don't forget to import...\n\
> -import org.kde.plasma.core 2.0 as PlasmaCore\n\
> -*/\n\
> -    PlasmaCore.IconItem {\n\
> -        source: \"" + preview.iconName + "\"\n\
> -        Layout.preferredWidth: units.iconSizes.medium\n\
> -        Layout.preferredHeight: units.iconSizes.medium\n\
> -    }\n\
> -";
> -                clipboard(code);
> +            GridLayout {
> +                anchors.centerIn: parent

Use a Kirigami.FormLayout for this (if not using it was deliberate because of 
layout bugs, let's fix those)

> Preview.qml:91
> +                }
> +                ToolButton {
> +                    icon.name: "edit-copy"

Needs a tooltip saying that this will copy the name to the clipboard (the icon 
isn't enough)

> Preview.qml:94
> +                    text: preview.iconName
> +                    onClicked: clipboard(preview.iconName)
> +                }

This is going to shock people, but I might even suggest displaying a Kirigami 
PassiveNotification saying "<text> copied to clipboard" when this is clicked. 
Otherwise it looks like nothing happens when you click the button.

> Preview.qml:101
> +                }
> +                ToolButton {
> +                    icon.name: "document-open"

Needs a tooltip saying that this will open the image in an external app (the 
icon isn't enough, though making it actually use the icon of the app that it 
will open with would help with that)

> Tools.qml:23
>  import QtQuick 2.2
> -import QtQuick.Controls 2.0
> +import QtQuick.Controls 2.5
>  import QtQuick.Layouts 1.0

`import QtQuick.Controls 2.5 as QQC2` (See T10862 
<https://phabricator.kde.org/T10862>)

> Tools.qml:94
>  
> -        PlasmaComponents.TextField {
> +        TextField {
>              id: pixelSizeInput

Let's just use a label for this. There's no value to letting people enter 
arbitrary numbers here.

> Tools.qml:114
> +    }
> +    Rectangle {
> +        opacity: 0.2

You could potentially use a Kirigami.Separator for this

> cuttlefish.qml:24
>  import QtQuick.Layouts 1.0
> +import QtQuick.Controls 2.5
>  

ditto

REPOSITORY
  R118 Plasma SDK

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

To: cblack, #vdg
Cc: ndavis, filipf, davidedmundson, ngraham, plasma-devel, #vdg, 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