subdiff added inline comments.

INLINE COMMENTS

> hein wrote in ContextMenu.qml:59
> That's kinda what I was wondering. If the context menu is opened for an 
> element in the tooltip, why is the visualParent the task item?

It's basically the second best solution. ;)

When I tried to set the tooltip as visual parent, it opened the context menu at 
the border of the tooltip how it is expected, but it also closed the tooltip, 
because the context menu requested focus. So the context menu is suddenly 
standing in the middle of the screen without being next to the grouped task.

If you know a way how the context menu can be opened at the border of the 
grouped task while not hiding the tooltip automatically, I would gladly 
implement this solution instead.

> anthonyfieroni wrote in ToolTipDelegate.qml:199
> Why we want to make tooltip smaller? Your patch will effect all screens with 
> higher resolution than your i.e. will be smaller to read. Do not use magic 
> number, get it  pro rata of screen resolution. You must apply your patch to 
> FullHD or bigger to see the difference.

The font sizes are directly based on `theme.mSize(theme.defaultFont).height`, 
which scales with default font size and screen DPI.
See here: 
https://api.kde.org/frameworks/plasma-framework/html/classPlasma_1_1Theme.html#a7d8a89c8f7c04382c229600c4f5d934a

> anthonyfieroni wrote in ToolTipDelegate.qml:237
> Again, do not make a bad decisions, use Screen width, height and pixel ratio 
> for HiDPI.
> 
>   import QtQuick.Window 2.2
>   property int maxWidth: Screen.width / Screen.devicePixelRatio
>   property int maxHeight: Screen.height / Screen.devicePixelRatio

`header.width`, i.e. `thumbnail.width` is controlled by a constant factor of 
`theme.mSize(theme.defaultFont).width`, which again is font and screen DPI 
dependent, such that the thumbnail will always have the width of the maximal 
possible font and DPI dependent width of the app name in the header plus close 
the button width.

Directly using `Screen.width / ...` doesn't make much sense to me. We want the 
thumbnail to fill out the available space defined by the header above it.

The `height` variable in contrast is arbitrary and could be any value, which 
would always make somehow sense since windows can have any format by resizing 
them. Using the Screen height-width ratio on `heider.width` could be also a 
solution, but I'm not sure if people, who use their monitor in potrait mode 
want their thumbnails to be suddenly a lot higher than on the normal screen 
next to it, which also means that the tooltip needs more space on the portrait 
mode screen, since we cannot reduce the width on the other hand (needs 
horizontal space for app name / window title).

REPOSITORY
  R119 Plasma Desktop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: subdiff, #vdg, hein, #plasma
Cc: anthonyfieroni, hein, colomar, plasma-devel, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas

Reply via email to