Panel should not stop auto-hiding if a window wants attention
Needs ReviewPublic

Authored by michaelmoon on May 16 2018, 5:42 AM.

Details

Summary

BUG: 394119

I love an auto-hiding panel.

However, if any window wants attention, the panel will *never* hide until I have given focus to each and every attention-seeking window across all desktops. This infuriates me.

The attached patch adds a configuration option to fix this behaviour, so the panel will only appear when I mouse over its activation area or it's being configured, but not if windows want attention.

Test Plan

in konsole, sleep 3; false, then switch desktop. (or any other action that makes a window "want attention")

With added option (default) enabled, panel unhides and remains until konsole is focused. This is the current behaviour.

With added option disabled, panel does not pop out, however window is still highlighted, and konsole's pop-over notification still works as expected. This is the desired behaviour.

Diff Detail

Repository
R119 Plasma Desktop
Lint
Lint Skipped
Unit
Unit Tests Skipped
michaelmoon created this revision.May 16 2018, 5:42 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 16 2018, 5:42 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
michaelmoon requested review of this revision.May 16 2018, 5:42 AM
davidedmundson requested changes to this revision.May 16 2018, 6:00 AM
davidedmundson added a subscriber: davidedmundson.

But the window is marked as needs attention because it needs attention. We can't mod that.

(Fwiw, better fix for you locally will be to mod the task manager to never set its status to needs attention, this patch mods handling of all plasmoids)

This revision now requires changes to proceed.May 16 2018, 6:00 AM
michaelmoon edited the summary of this revision. (Show Details)
michaelmoon edited the test plan for this revision. (Show Details)
michaelmoon changed the repository for this revision from R120 Plasma Workspace to R119 Plasma Desktop.

Add an option to prevent Task Manager unhiding panels when a window wants attention. Default is existing behaviour.

See discussion in https://bugs.kde.org/show_bug.cgi?id=394119

This patch doesn't apply due to some path weirdness:

Checking patch plasma-desktop-5.15.5_old/applets/taskmanager/package/contents/ui/main.qml => applets/taskmanager/package/contents/ui/main.qml...
error: plasma-desktop-5.15.5_old/applets/taskmanager/package/contents/ui/main.qml: does not exist in index

Instead of doing a manual diff between two files (old and new), you'd be better off editing the files in the plasma-desktop git repo and then running git diff to get your diff for this.

Also make sure you're working on git master, since that where the patch will be merged to once accepted.

Altered patch to apply against plasma-desktop:b2ce4bd (current git master)

davidedmundson resigned from this revision.Jul 16 2019, 9:52 AM

No objection from me.

I'll wait a few days in case Eike has comments, but from my POV, ship it

ngraham accepted this revision.Jul 16 2019, 4:56 PM
ngraham added a subscriber: hein.
This revision is now accepted and ready to land.Jul 16 2019, 4:56 PM
hein added a comment.EditedJul 22 2019, 3:57 PM

I don't like this.

  • I think it's option creep
  • The user would have to configure this for each of their Task Manager applets to make it pervasive (consider multiple monitors with multiple Task Managers), but it's unlikely the user wants it to be different across them, so this isn't the most convenient way to do it
  • It won't address other applets that may use the same status, and adding this to the TM puts a burden on applet authors to add the same feature, which increases the barrier to entry to make a good applet. This overhead can be avoided
  • The conceptual problems aside, the option naming and UI string also aren't good - the Task Manager doesn't know anything about being in a panel, but hiding/unhiding is a panel feature, this shouldn't poke through. It'd have to be something more generic about the attention status

I think a better approach to this problem would be to make the panels aware of the new Do Not Disturb mode feature introduced in 5.16, and if necessary add some config to what DND affects, or to make it panel-wide config. Or just in general, since an applet calling for attention is a notification feature, I think this configuration is better placed globally in notification settings.

Something that *could* be made configurable is whether to only take the current desktop into account, though.

hein requested changes to this revision.Jul 22 2019, 3:57 PM
This revision now requires changes to proceed.Jul 22 2019, 3:57 PM

@hein thanks for your comments.

Fwiw, the reason I use KDE in the first place is that it has all the options, and I can control my desktop experience to a significantly greater degree than any other DE I've tried so far - but there's just one option it seems to be missing, to my great frustration (and several others, see bug 394119)!

I'm a fairly experienced programmer, but mostly in embedded rather than graphical, let alone Qt/KDE specifics, so any hints on where specifically to look for places to inject the ability to stop Panels getting in my way when some random stupid app sets its attention flag would be deeply appreciated.

As you can see in the history, I originally submitted this at the Panel level rather than the Task Manager level, although admittedly with a clumsy hammer - would you prefer if it were moved back to the panel? @davidedmundson listed some reasons that this might be inappropriate in bug 394119, which is why I moved the behaviour change from Panel to Task Manager in the first place.

Are you suggesting that I would need to set KDE permanently into this 'Do Not Disturb' mode to avoid one single frustrating behaviour? What other features might I lose access to in that case?

Global setting in notification settings sounds fine, any hints on where to find those settings, and how to plumb it into the Task Manager or Panel? However I'm somewhat concerned that this may also conceptually be the wrong spot since the core issue (see bug 394119) specifically and uniquely affects the interaction between Task Manager's handling of random windows' rogue flag-setting, and Panels acting on the results of that.

In short, where exactly should this option go and what should it be called, given the conceptual structure of KDE as a whole? All I want is for panels containing a task manager to not *stay* visible if some application randomly decides it needs attention..

hein added a comment.EditedJul 22 2019, 5:11 PM

I wonder if there's another fix we could attempt: Rather than permanently staying visible, would having to move your mouse over the panel and exiting again to hide it again work? Maybe implementing code for that in the panel containment could be a config-less hammer we could take to the thing ...

would having to move your mouse over the panel and exiting again to hide it again work?

Like it did in KDE4? Yeah that would work for me!

Does that make the associated bug a regression?

@eike please also see my comments in the bug report for context.


The core part that's up for discussion is simply:
"does the task manager need attention simply because some different window needs attention"?

Semantically the scope of that question is purely within the task manager, and not a wider panel problem.

If we want to avoid option creep, we could simply re-evaluate that.

I agree the text isn't great, but I didn't have anything else that didn't expose internals.

hein added a comment.Jul 22 2019, 5:45 PM

If that's how it worked in KDE 4 I think it could well qualify as a regression.

Regardless of that, my personal take would be that the user entering and exiting the panel constitutes having explicitly acknowledged "needs attention" status and then ignoring it after is fine.

Implementation-wise, that means in the part where you removed that block earlier we need to evaluate a dirty flag for whether the status change has been acknowledged with an enter+leave or not.

hein added a comment.EditedJul 22 2019, 6:06 PM

"does the task manager need attention simply because some different window needs attention"?

There's an easy answer: We have a lot of users who rely on that.

A lot of people want to be able to focus without disturbance, but a lot of people also don't want to miss that important notification they're waiting for. For the latter, having an off-desktop window be able to punch through and raise the panel is important.

If you look at industry trends, there's currently a very active discussion on how to mediate the range of "leave me alone, all these notifications are killing me" and "let the important stuff punch through". In 5.16 we introduced some of the same ideas others have: DND mode and per-app configurability.

This problem is broadly part of the same conversation and so it's not really specific to the TM or the panel. What the TM does is provide a view into system state sharded by application and window. The notification applet does the same. The window attention state and notification messages are both notification mechanisms. Some of the "can this punch through?" configurability should likey be there, because most definitely some things should be able to.

The other angle to attack this problem from is purely the panel UX, where you can pick between the model of "attention is needed permanently until nothing needs attention" and a model of "requests for attention need to be explicitly acknowledged, and after they have been can be ignored", i.e. allowing the panel to hide after enter+exit clears a bit. I like this one, also because it's totally something a containment can decide for itself from the Plasma conceptual POV.

Is there a conclusion here, or a plan of attack for @michaelmoon to revise the patch?

I'd love to know as well.. While I've got my patches in my local patch set, others in the linked bug are just fighting with their panel every day.

If there's a tracking bug for 5.16 or 5.17 release, perhaps we could add this to its dependencies?

Updated for 5.16.5, which the previous patch failed against.