[applets/taskmanager] Make the hover state clarifying, but not main
ClosedPublic

Authored by epopov on Thu, Mar 26, 11:52 AM.

Details

Summary

I'm trying to make a custom appearance of the Task Manager by creating a new theme, but I ran into some issue. I expected that there are three main states (minimized, normal and active) and one clarifying state (hover). But I was surprised to find that there are four main states (minimized, normal, active and hover), and the hover state is also main, but not clarifying. I believe that the hover state should be clarifying for the main states, i.e. there must be three main states (minimized, normal and active) and three additional (minimzed-hover, normal-hover, active-hover).

BUG: 418318

See https://phabricator.kde.org/P555

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
epopov created this revision.Thu, Mar 26, 11:52 AM
Restricted Application added a project: Plasma. · View Herald TranscriptThu, Mar 26, 11:52 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
epopov requested review of this revision.Thu, Mar 26, 11:52 AM

Thanks for the patch! For visual patches like this, it's helpful if you put before-and-after images or screen recordings in the Test Plan section.

@ngraham the default theme doesn't change with this patch.

I believe there are no plans to do so.

applets/taskmanager/package/contents/ui/Task.qml
331

this is a bit longwinded, can we make a function in TasksTools?

broulik added inline comments.
applets/taskmanager/package/contents/ui/Task.qml
331

We can use ECMAScript 6 now, use spread syntax:

return [
    ...TaskTools.taskPrefix(etc),
    ...foo,
    ...bar
];

Note the ...

epopov updated this revision to Diff 78568.Thu, Mar 26, 4:26 PM

Create a new function in TasksTools to make the code more readable.

epopov updated this revision to Diff 78580.Thu, Mar 26, 6:18 PM

Use ES6 spread syntax instead of Array.concat()

davidedmundson accepted this revision.Thu, Mar 26, 6:21 PM
This revision is now accepted and ready to land.Thu, Mar 26, 6:21 PM
epopov marked 2 inline comments as done.Thu, Mar 26, 6:31 PM
epopov added inline comments.
applets/taskmanager/package/contents/ui/Task.qml
331

TaskTools.taskPrefix(...) returns an array, so it will be an array-of-arrays

331

Ah, now I understand. It looks amazing.

epopov marked 3 inline comments as done.Thu, Mar 26, 6:32 PM

This does introduce a visual change: the hover effect now adds a blue background, even for un-launched pinned launchers. Currently the hover effect for pinned launchers is just a lightening of the icon itself.

I'm not against it, but this is a visual change that must be discussed; this isn't just a bugfix or code cleanup.

epopov updated this revision to Diff 78599.Thu, Mar 26, 11:13 PM

Fix hover state for pinned launchers

ngraham retitled this revision from Make the hover state clarifying, but not main to [applets/taskmanager] Make the hover state clarifying, but not main.Fri, Mar 27, 4:08 AM
This revision was automatically updated to reflect the committed changes.

Thank you, guys!