Makes it more obvious there's a menu with more options.
You can still right-click the thumbnail area as before.
Details
Details
- Reviewers
ngraham romangg - Group Reviewers
Plasma VDG - Commits
- R120:d7fbe6c86377: [Notifications] Add visible menu button to thumbnail strip
This avoids D7971
Unfortunately since the actions are asynchronously added to the menu from what I can tell, right aligning it to the menu button doesn't fully work.
Diff Detail
Diff Detail
- Repository
- R120 Plasma Workspace
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Love the idea and the feature. Not sure I'm a fan of making this a hamburger menu with no text, and putting it inside the preview. My preference would be to put it right under the existing Open button and give it a label ("Menu/Options/More/something else"). Let's see what VDG has to say.
Comment Actions
put it right under the existing Open button
That'll make the code more complex as currently the ThumbnailStrip.qml is fully self-contained. Also that won't really work for multiple thumbnails further complicating things.
Comment Actions
Or should we remove the "Öffnen/Open" button? Left click on a preview image / item (with tooltip what will happen) would then do this "default action". Right click (or click on the hamburger menu) would show alternative options in the menu.
Comment Actions
You can already right-click on the image to pull up the menu. The problem is that it's a hidden UI; many will miss it because they won't think to right-click on the image.
Could we maybe make it a separated button under Open only for the common case where the notification only displays one image in it? Then if the notification has several, then we could badge each with a hamburger button as per this change.
Comment Actions
I think you misunderstood what I was saying: remove the default action button, instead do this via a left click on the respective item/preview. And right click and burger menu for menu as per this patch here.
I.e. for this patch here: +1
Comment Actions
I'm not sure we should remove the Open button. That seems like it would be going backwards in the usability department, because again, a lot of people won't think to click on the picture. A visible button that looks like a button and has a word in the imperitive mood says "click me to do a thing!" A picture doesn't.
Comment Actions
+1 for this patch. The three-line buttons happen in many other places too (dolphin, window title bars, firefox, chrome, etc.). Users quickly learn that pattern and recognize it as the "menu button" so I don't think that would be a problem.
A possible tweak for this would be to replace the open button with "show folder", and make the thumbnail open the image. At least for the use-case of screenshots, opening the folder is way more useful since you usually want to send the file to someone (and you can drag and drop it on telegram, kmail, etc.).
Comment Actions
Yes, that's exactly my use case too, most of the time. I made a separate patch for that a few months ago: D7971
Comment Actions
After thinking about it for a while, since this is a more general patch for all notifications with previews, I think it can be tracked separately from the default set of buttons that Spectacle displays. That is, even with this, we could potentially still land D7971 as a further usability improvement for the use case for Spectacle where you just really want to view the file in your file manager ASAP. So I'm removing my objections.
| applets/notifications/package/contents/ui/ThumbnailStrip.qml | ||
|---|---|---|
| 113–114 | 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. | |
| 233 | The hamburger menu lines are wide stretched in my scaling 2:  This could be the same problem we had with the ones of plasma-pa. Maybe the solution we did there could apply here too. | |
| applets/notifications/plugin/thumbnailer.cpp | ||
| 189 | 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. | |
Comment Actions
Or should we remove the "Öffnen/Open" button?
Good idea. However, the "Open" button is created by the application itself and there's no way of knowing to the application whether the notification server supports that thumbnail strip that is proprietary to Plasma and non-standard. While FDO specifies a "Capabilities" property that a client can query to see what it supports, that isn't accessible through KNotifications. In short: if we remove the "Open" button on other desktops the user won't get any way of accessing the screenshot.
| applets/notifications/package/contents/ui/ThumbnailStrip.qml | ||
|---|---|---|
| 113–114 | The thumbnail has *always* been clickable. The open hand cursor is merely an indicator to show that it's draggable, the very reason this feature exists. | |
| applets/notifications/plugin/thumbnailer.cpp | ||
| 189 | The problem is not the placing but the fact that the actions are populated asynchronously, you can observe the same in Dolphin where the context menu sometimes (very rarely, actually) shows up detached from the cursor position. | |
| applets/notifications/package/contents/ui/ThumbnailStrip.qml | ||
|---|---|---|
| 113–114 |
Right, I overlooked this.
Is it used somewhere else this way in the workspace? I checked:
Imo if an item is dragabble and clickable it should be a normal cursor. Since it is not directly related to this patch about the context menu anymore though, we can leave it like it is for now. | |
Comment Actions
I'll give this an ok, since the base functionality is there. I'm not sure if I understood the reasoning completely why the context menu can't be aligned correctly to the context menu button (something asynchronously, but the final position of the menu button is known at some point for sure), but maybe this can be fixed later. Same for the graphical issue with stretched lines.