romangg requested changes to this revision.
romangg added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> ThumbnailStrip.qml:113
>          preventStealing: true
>          cursorShape: Qt.OpenHandCursor
> +        acceptedButtons: Qt.LeftButton | Qt.RightButton

Since now the thumbnail is clickable, remove.

Also could you give the thumbnail an effect to show that it is clickable? Best 
would be imo a frame in highlight color, which switches to the context menu 
button, when mouse cursor moves from thumbnail to inside of context menu button 
area. This would be consistent with other elements of the workspace (with 
breeze).

But since we have not yet decided on a consistent highlighting scheme in 
general, you can also try other styles.

> ThumbnailStrip.qml:233
> +            PlasmaCore.IconItem {
> +                anchors.fill: parent
> +                anchors.margins: parent.padding

The hamburger menu lines are wide stretched in my scaling 2:
F5537098: Screenshot_20171209_231619.png <https://phabricator.kde.org/F5537098>
This could be the same problem we had with the ones of plasma-pa. Maybe the 
solution we did there could apply here too.

> thumbnailer.cpp:189
> +
> +        pos = ctx->mapToGlobal(QPointF(0, ctx->height())).toPoint();
> +

The following code positions the menu according to the button. But it still 
covers half of it. The context menu should not cover the button at all. Can you 
somehow use PlasmaComponents.Menu? It already has suitable placement code 
covering special cases (not enough space etc.).

Otherwise copy the placement code from its C++ part and use it here.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, #vdg, ngraham, romangg
Cc: januz, romangg, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

Reply via email to