D27684: [applets/taskmanager] Show PA-related features even when audio indicators are disabled

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  Good stuff.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  show-mute-feature-even-when-audio-indicators-are-disabled (branched from 
master)

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

To: ngraham, #plasma, hein
Cc: hein, gvgeo, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27182: [applets/taskmanager] Clean up Tooltip code

[Eike Hein]

hein added a comment.


  trmdi's point is good, otherwise I like it.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  clean-up-tooltip-code (branched from master)

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

To: ngraham, #plasma, #vdg, ndavis
Cc: hein, trmdi, broulik, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26582: Kicker/RecentUsageModel: Allow to open smb:/ sftp:/ resources

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  Thanks :)

REPOSITORY
  R120 Plasma Workspace

BRANCH
  arcpatch-D26582

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

To: meven, #plasma, broulik, aleksejshilin, hein
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26582: Kicker/RecentUsageModel: Allow to open smb:/ sftp:/ resources

[Eike Hein]

hein added a comment.


  So does this fall through to the dialog when the type hasn't been determined?

REPOSITORY
  R120 Plasma Workspace

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

To: meven, #plasma, broulik, aleksejshilin, hein
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25428: Improved support for desktop "Folder View"

[Eike Hein]

hein added a comment.


  Just a quick note I will review this tomorrow or next week when I'm on 
vacation, sorry for the long wait.

REPOSITORY
  R119 Plasma Desktop

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

To: steffenh, #plasma, hein, ngraham, mart
Cc: mart, ngraham, nicolasfella, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D26580: Kicker/RecentDocuments display place's icon when resource matches one

[Eike Hein]

hein added a comment.


  This patch is fine and in line with the existing implementation, but it 
occurs to me we probably should make the RecentUsageModel use FileEntry and put 
it centrally there at some point.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: meven, #plasma, ngraham, hein
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26386: Kicker/RecentDocuments: add icons to actions

[Eike Hein]

hein accepted this revision.
hein added a comment.


  Good stuff!

REPOSITORY
  R120 Plasma Workspace

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

To: meven, #plasma, ngraham, hein
Cc: hein, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26378: [Download Plugin] Register downloaded files as recent documents

[Eike Hein]

hein accepted this revision.
hein added a comment.


  I love it.

REPOSITORY
  R856 Plasma Browser Integration

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

To: broulik, #plasma, hein, #vdg, fvogt, ngraham
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26385: Kicker/RecentDocuments Avoid referring to hidden places

[Eike Hein]

hein added a comment.


  Thanks!
  
  We can revisit it after some more discussion.

REPOSITORY
  R120 Plasma Workspace

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

To: meven, #plasma, ngraham, hein
Cc: hein, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26385: Kicker/RecentDocuments Avoid referring to hidden places

[Eike Hein]

hein added a comment.


  I don't agree with this patch. Hiding a place or section from the Places 
panel doesn't mean the user didn't add it and it's not to be used. We don't 
hide it from Folder View's config or relative title generator either. Hidden is 
not removed. Please revert.

REPOSITORY
  R120 Plasma Workspace

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

To: meven, #plasma, ngraham
Cc: hein, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26386: Kicker/RecentDocuments: add icons to actions

[Eike Hein]

hein requested changes to this revision.
hein added a comment.
This revision now requires changes to proceed.


  It should be `iconName` after `label`.

REPOSITORY
  R120 Plasma Workspace

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

To: meven, #plasma, ngraham, hein
Cc: hein, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25773: [Applets/Task Manager] Add pinned filemanager and browser apps by default

[Eike Hein]

hein added a comment.


  It's definitely a discussion I'd enjoy more :)

REPOSITORY
  R119 Plasma Desktop

BRANCH
  some-apps-by-default (branched from master)

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

To: ngraham, #plasma, broulik, #vdg, hein, filipf, ndavis
Cc: filipf, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25773: [Applets/Task Manager] Add pinned filemanager and browser apps by default

[Eike Hein]

hein accepted this revision.
hein added a comment.


  Ah sorry, I had a reply typed into the text field but must have botched 
hitting Submit.
  
  Personally, I'm not a fan - I think there's a bad trend of us overstuffing 
the default panel now like we're looking out of "what more can we add ..." out 
of idleness. I predict we'll remove things like this and the Show Desktop 
applet again, when the next collapsing-under-its-own-weight cycle inevitably 
hits. To speed that along I'll accept ;)

REPOSITORY
  R119 Plasma Desktop

BRANCH
  some-apps-by-default (branched from master)

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

To: ngraham, #plasma, broulik, #vdg, hein, filipf, ndavis
Cc: filipf, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25762: Only serialize window pixmap if we're using it

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  Nice :)

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, hein
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26147: Kickoff computer tab : don't display filepath for places in description

[Eike Hein]

hein requested changes to this revision.
hein added a comment.
This revision now requires changes to proceed.


  The `return QString()` shouldn't be necessary if we just fall through to the 
final return.

REPOSITORY
  R120 Plasma Workspace

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

To: meven, ngraham, #plasma, #vdg, hein
Cc: hein, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25874: [Task Manager] Consider parent PID for audio stream

[Eike Hein]

hein added a comment.


  Just as a sanity check: libtm knows the pid for a window. Have you checked if 
there might be a faster way of doing this by checking if the stream pid is 
among its children?

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, hein
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25762: Don't bother serializing window icon pixmap for known services

[Eike Hein]

hein added a comment.


  Ah right, I didn't look at the code context and forgot we don't just load the 
custom pixmap in the same function body but also in Private::icon. But we also 
do this there:
  
  `usingFallbackIcon.insert(window)`
  
  That means you can limit serializing to cases where the window is in 
`usingFallbackIcon`.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, hein
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25762: Don't bother serializing window icon pixmap for known services

[Eike Hein]

hein added a comment.


  If the concern is that QIcon::name isn't good enough but we actually have an 
icon inside, why don't we just change the check to QIcon::isNull?

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, hein
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D23242: Add grouping and "Show Only Minimized" options to icons-only task manager

[Eike Hein]

hein added a comment.


  Let's go for it, it's fairly in line with the TM applet refectoring we 
planned in Valencia at the end of the day

REPOSITORY
  R119 Plasma Desktop

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

To: GB_2, #plasma, #vdg, hein, ngraham
Cc: ngraham, hein, #vdg, plasma-devel, #plasma, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D23039: Make Kickoff restore favorites order when dragging an item to desktop

[Eike Hein]

hein accepted this revision.
hein added a comment.


  Good one, thanks!

REPOSITORY
  R119 Plasma Desktop

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

To: lisin, #plasma, ngraham, hein
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25773: [Applets/Task Manager] Add pinned filemanager and browser apps by default

[Eike Hein]

hein added a comment.


  I don't really understand why the panel being bare by default is a problem. 
Isn't it nicer when users don't have to opt out of clutter like this?

REPOSITORY
  R119 Plasma Desktop

BRANCH
  some-apps-by-default (branched from master)

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

To: ngraham, #plasma, broulik, #vdg, hein, filipf, ndavis
Cc: filipf, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D22053: [libtaskmanager] Track stacking order and window activation (on X11)

[Eike Hein]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R120:c0acd1434147: [libtaskmanager] Track stacking order and 
window activation (on X11) (authored by hein).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22053?vs=60524=70313

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

AFFECTED FILES
  libtaskmanager/abstracttasksmodel.h
  libtaskmanager/taskgroupingproxymodel.cpp
  libtaskmanager/xwindowtasksmodel.cpp

To: hein, #plasma
Cc: davidedmundson, pino, anthonyfieroni, ngraham, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D25055: [containments/desktop] Delete the "Tweaks" page and its options

[Eike Hein]

hein added a comment.


  If we want to keep the handles-on-hover outside of Edit Mode, is there a 
reason it can't be always on and needs to be gated behind a checkbox?
  
  I have to admit though I have a hard time seeing users like/rely on this 
behavior. Hovering and just waiting, even once you've learned it's possible, is 
always going to be an unsatisfying user experience - you're literally waiting 
around until the system deigns to approve your intent (long-press, in contrast 
is still //active// to some extent). I'd be worried that some users might learn 
"long hover" as "this is how it's supposed to be done" and then be forever 
annoyed that widgets are so cumbersome to handle. I'd propose removing it and 
fully leading users towards Edit Mode instead?

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, mart, GB_2, #plasma, hein
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D19822: [Task Manager] Toggle mute when the audio indicator is clicked

[Eike Hein]

hein requested changes to this revision.
hein added a comment.
This revision now requires changes to proceed.


  Approach is fine, but please move the MouseArea into the 
audioStreamIconLoader's source component so it's not always instantiated even 
when it's not needed. :)

REPOSITORY
  R119 Plasma Desktop

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

To: faridb, #plasma, #vdg, ngraham, hein, GB_2
Cc: GB_2, mart, hein, filipf, mvourlakos, ngraham, broulik, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra

D14796: Port Keyboard input modules and Expand scope to cover input method configuration

[Eike Hein]

hein added a comment.


  Ping @gpark, are you available to resume work on this too? There's a growing 
amount of interest which means more dev hands to aid the effort.

REPOSITORY
  R119 Plasma Desktop

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

To: gpark, hein
Cc: davideichelsdoerfer, yerilov, kucharczyk, lavender, ngraham, apol, 
plasma-devel, hein, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, ahiemstra, mart

D24454: Include an emoji picker

[Eike Hein]

hein added a comment.


  If I understand right, the emoji workflow this implements is via copy-paste, 
it can't directly insert into the text fields via the IME bus. Do you have a 
plan for that?

REPOSITORY
  R119 Plasma Desktop

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

To: apol, #plasma, #vdg
Cc: hein, #vdg, GB_2, mart, ngraham, davidedmundson, broulik, plasma-devel, 
LeGast00n, The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, 
Ghost6, jraleigh, MrPepe, fbampaloukas, squeakypancakes, alexde, 
IohannesPetros, trickyricky26, ragreen, mglb, crozbo, ndavis, ZrenBot, firef, 
alexeymin, skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
aaronhoneycutt, abetts, sebas, apol, ahiemstra, mbohlender

D24503: Integrate the normal keyboard layout switcher

[Eike Hein]

hein added a comment.


  It's a small part of the work Geon Park did in his GSoC project to refactor 
kimpanel and our input sources management in general, but in a more 
merge-friendly format.
  
  The former sadly never got any review attention because the patch was too 
big, so maybe this is the right way to make progress.
  
  For context though, in the GSoC the following work was done:
  
  - The keyboard layout KCM was rewritten in Qt Quick
  - It's based on a model that can contain both IMEs and keyboard layouts
  - The model can dynamically react to IME daemons coming and going and doing a 
sensible fallback
  - The implementation is shared between the KCM, kimpanel (which, like here, 
replaces the old SNI) and the kded module that on X11 provides the D-Bus API to 
manage layouts
  - That means global shortcuts like Next/Prev Keyboard Layout we have work 
over both IMEs and layouts
  - That means the OSD and the lock screen show the right thing and became 
IME-aware
  
  And a couple of other things. What wasn't done was touch KWin/Wayland (which 
in a Plasma/Wayland session provides the same D-Bus API kded provides on X11).
  
  So it's important to remember that this is the goal. With this patch Plasma 
would get better, but it doesn't address the whole problem (System Settings 
being useless for IMEs, the IME daemon making System Settings useless when 
running, two incompatible systems fighting each other, expert knowledge 
required to set things up in the first place, etc).

REPOSITORY
  R119 Plasma Desktop

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

To: apol, #plasma
Cc: hein, broulik, davidedmundson, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D23728: [kicker/kickoff] Use icon from appstream handler

[Eike Hein]

hein added a comment.


  Thanks!

REPOSITORY
  R119 Plasma Desktop

BRANCH
  appstream

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

To: nicolasfella, hein
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D23640: [kicker/kickoff] Add icon to hide application action

[Eike Hein]

hein added a comment.


  Would it be better to add a new icon name to the theme instead of starting to 
use semantically dubious icons?

REPOSITORY
  R119 Plasma Desktop

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

To: nicolasfella, #plasma, hein, ngraham
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23623: [kicker/kickoff] Use discover icon for manage application action

[Eike Hein]

hein reopened this revision.
hein added a comment.
This revision is now accepted and ready to land.


  Distros can change the package manager to be run, so hard-coding the icon is 
wrong. It should be looked up from the configured application.

REPOSITORY
  R119 Plasma Desktop

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

To: nicolasfella, #plasma, hein, ngraham
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23652: [Kicker/Kickoff] Re-word "Manage ..." menu item for greater clarity

[Eike Hein]

hein added a comment.


  It was originally "Uninstall", then got renamed into "Manage" when the 
implementation changed to run the package manager (changable by the ISV) which 
can do different actions too.

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, #vdg, #plasma, hein, nicolasfella
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23677: [Folder View] Load icon only for isPopup case

[Eike Hein]

hein requested changes to this revision.
hein added a comment.
This revision now requires changes to proceed.


  Hmm, I'm not a fan. That only the isPopup case ends up showing the icon in 
the shell is something you and I know from the outside, but I don't think a 
widget should be making that assumption. Otherwise we bake implementation 
details of the containents into widget code, and if we ever change the former 
we have to fix the widgets. Take for example a widget list like the one bandied 
about for config modes on the sprint.

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, hein
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D23356: Add ksysguard plugin for taskmanager integration

[Eike Hein]

hein added a comment.


  Do you really need a TasksModel, or is a WindowTasksModel enough?

REPOSITORY
  R106 KSysguard

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

To: davidedmundson, #plasma
Cc: hein, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D23242: Add grouping and "Show Only Minimized" options to icons-only task manager

[Eike Hein]

hein added a comment.


  Code's fine, but let's discuss the change briefly - disabling grouping in ITM 
gets you a bunch of identical-looking icons you can't tell apart. I think 
that's a pretty ugly user experience, does it make sense to have an option to 
configure the applet into an invariably sort of unpleasant to use state? That's 
why I resisted it in the past.

REPOSITORY
  R119 Plasma Desktop

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

To: GB_2, #plasma, #vdg, hein, ngraham
Cc: ngraham, hein, #vdg, plasma-devel, #plasma, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D23241: Show application window icon on AboutPage

[Eike Hein]

hein added a comment.


  I was looking into this recently and added the component named getter, which 
does the trick for me and is much nicer to use than setting the program logo. 
So in some sense this is not strictly necessary, and also it would really be 
better if the window icon first part of the Qt.application upstream API in QML. 
It's still an okay workaround though. Let's see what Marco thinks.

REPOSITORY
  R169 Kirigami

BRANCH
  about_icon (branched from master)

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

To: caspermeijn, mart, apol, #kirigami, hein
Cc: ngraham, plasma-devel, fbampaloukas, domson, dkardarakos, apol, 
davidedmundson, mart, hein

D22960: Add handle icons

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:29dffaf4cd72: Add handle icons (authored by hein).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22960?vs=63155=63327

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

AFFECTED FILES
  KF5Kirigami2Macros.cmake

To: hein, #frameworks, apol, mart
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22866: [Kickoff] Always select previous category in Applications View when navigating backwards

[Eike Hein]

hein accepted this revision.
hein added a comment.


  Good idea!

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff-appview-remember-previous-category (branched from master)

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

To: akozlovskiy, #plasma, ngraham, hein
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22988: Fix incorrect Kickoff tab bar layout for vertical panels

[Eike Hein]

hein added a comment.


  ... I'm not the author or maintainer of this code, but I had a look anyway :).
  
  @lisin, I agree with you that the sizing bug should be fixed in TabBar 
instead, in plasma-frameworks.git. It's very unorthodox to call a property 
change handler as a function, and it's not going to fix this for other 
potential users of the component.
  
  Otherwise the patch looks good. Could you resubmit it without line 435, and 
then submit a seperate patch to plasma-frameworks?

REPOSITORY
  R119 Plasma Desktop

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

To: lisin, #plasma, hein, ngraham
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22960: Add handle icons

[Eike Hein]

hein created this revision.
hein added reviewers: Frameworks, apol, mart.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
hein requested review of this revision.

REPOSITORY
  R169 Kirigami

BRANCH
  master

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

AFFECTED FILES
  KF5Kirigami2Macros.cmake

To: hein, #frameworks, apol, mart
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22955: Add icon used by Kirigami.AboutPage

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:c17c048ec2f7: Add icon used by Kirigami.AboutPage 
(authored by hein).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22955?vs=63146=63152

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

AFFECTED FILES
  KF5Kirigami2Macros.cmake

To: hein, #frameworks, mart, apol
Cc: mart, plasma-devel, fbampaloukas, domson, dkardarakos, apol, 
davidedmundson, hein

D22955: Add icon used by Kirigami.AboutPage

[Eike Hein]

hein created this revision.
hein added reviewers: Frameworks, mart.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
hein requested review of this revision.

REPOSITORY
  R169 Kirigami

BRANCH
  master

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

AFFECTED FILES
  KF5Kirigami2Macros.cmake

To: hein, #frameworks, mart
Cc: mart, plasma-devel, fbampaloukas, domson, dkardarakos, apol, 
davidedmundson, hein

D22901: Make alphabetic categorization of the "All Applications" section optional

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:258545f996cf: Make alphabetic categorization of the 
All Applications section optional (authored by hein).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22901?vs=62989=62992

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

AFFECTED FILES
  applets/kicker/package/contents/ui/main.qml
  applets/kicker/plugin/rootmodel.cpp
  applets/kicker/plugin/rootmodel.h

To: hein, #plasma, davidedmundson, tcanabrava
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22901: Make alphabetic categorization of the "All Applications" section optional

[Eike Hein]

hein created this revision.
hein added reviewers: Plasma, davidedmundson, tcanabrava.
Herald added a project: Plasma.
hein requested review of this revision.

REVISION SUMMARY
  Introduces a new prop and turns it off by default.
  
  Also rewrites the initial "all applications" data gathering avoid
  a crash uncovered while working on this (it didn't check EntryType)
  and reduces assumptions about the model topology.
  
  It also makes it collect root-level apps now, which was forgotten
  to be added when support for root-level apps were added.
  
  This supercedes D22762  and D22890 
. It does less duplicate work and fixes
  more bugs.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

AFFECTED FILES
  applets/kicker/package/contents/ui/main.qml
  applets/kicker/plugin/rootmodel.cpp
  applets/kicker/plugin/rootmodel.h

To: hein, #plasma, davidedmundson, tcanabrava
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22762: Take in consideration flat() for the All Applications menu

[Eike Hein]

hein added a comment.


  It's worth noting that this patch as-is also breaks the Dashboard UI ("All 
Applications" no longer shows content), so this revert wasn't just about 
implementation details but also actively broke upstream.
  
  I'm writing an alternative patch now.

REPOSITORY
  R119 Plasma Desktop

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

To: tcanabrava, davidedmundson, hein
Cc: hein, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22868: Revamp Kirigami.AboutPage

[Eike Hein]

hein added a comment.


  I'll remove that part of the patch later tonight and talk Marco into fixing 
it differently.

REPOSITORY
  R169 Kirigami

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

To: hein, #frameworks, #vdg, mart, apol, ngraham, leinir, nicolasfella
Cc: nicolasfella, ngraham, plasma-devel, fbampaloukas, domson, dkardarakos, 
apol, davidedmundson, mart, hein

D22867: Consistently use Units.toolTipDelay instead of hardcoded values

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:a99c14d1eac9: Consistently use Units.toolTipDelay instead 
of hardcoded values (authored by hein).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22867?vs=62879=62912

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

AFFECTED FILES
  src/controls/private/GlobalDrawerActionItem.qml
  src/controls/private/PrivateActionToolButton.qml
  src/controls/templates/SwipeListItem.qml
  src/controls/templates/private/BackButton.qml
  src/controls/templates/private/ForwardButton.qml

To: hein, #frameworks, mart
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22868: Revamp Kirigami.AboutPage

[Eike Hein]

hein updated this revision to Diff 62883.
hein added a comment.


  Don't hit Kirigami.Label (deprecated)

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22868?vs=62880=62883

BRANCH
  aboutpage

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

AFFECTED FILES
  src/controls/AboutPage.qml
  src/controls/Page.qml

To: hein, #frameworks, #vdg, mart, apol, ngraham, leinir
Cc: ngraham, plasma-devel, fbampaloukas, domson, dkardarakos, apol, 
davidedmundson, mart, hein

D22868: Revamp Kirigami.AboutPage

[Eike Hein]

hein added a comment.


  Discover:
  
  F7130049: Screenshot_20190801_100601.png 

  
  Material style:
  
  F7130053: Screenshot_20190801_100645.png 


REPOSITORY
  R169 Kirigami

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

To: hein, #frameworks, #vdg, mart, apol
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22868: Revamp Kirigami.AboutPage

[Eike Hein]

hein added a comment.


  Before:
  
  F7130036: Screenshot_20190801_15.png 

  
  After:
  
  F7130037: Screenshot_20190801_100033.png 


REPOSITORY
  R169 Kirigami

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

To: hein, #frameworks, #vdg, mart, apol
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22868: Revamp Kirigami.AboutPage

[Eike Hein]

hein created this revision.
hein added reviewers: Frameworks, VDG, mart, apol.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
hein requested review of this revision.

REVISION SUMMARY
  - Use a FormLayout to work nicely in both portrait and landscape orientations
  - Don't use an AbstractCard for the person delegate, it's very jarring in 
this design
  - Fix app description not having word wrap (breaks on portrait)
  - Improve inter-section spacing
  - Fix empty author email addresses showing as "Name <>"
  - Fix scrollbar length if a `Kirigami.Page` doesn't set any actions (and 
don't load a gigantic subtree of unnecessary objects into the page)
  - If an app uses `Kirigami.AbstractApplicationWindow` and doesn't have a 
`pageStack`, don't have the license delegate be a `LinkButton` that operates on 
one and causes an error
  - Add a fallback to KAboutData::componentName for the program icon - 
`KAboutData::programIconName` is deprecated, there's no QML accessor for its 
replacement `QGuiApplication::setWindowIcon` yet, but 
`KAboutData::programIconName` falls back to `componentName` already (it fails 
because `KAboutData::programIconName` isn't a qprop, but I don't feel like 
adding it to KCoreAddons for a deprecated thing while `componentName` is not 
deprecated)

REPOSITORY
  R169 Kirigami

BRANCH
  aboutpage

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

AFFECTED FILES
  src/controls/AboutPage.qml
  src/controls/Page.qml

To: hein, #frameworks, #vdg, mart, apol
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22867: Consistently use Units.toolTipDelay instead of hardcoded values

[Eike Hein]

hein created this revision.
hein added reviewers: Frameworks, mart.
Herald added a project: Kirigami.
hein requested review of this revision.

REPOSITORY
  R169 Kirigami

BRANCH
  master

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

AFFECTED FILES
  src/controls/private/GlobalDrawerActionItem.qml
  src/controls/private/PrivateActionToolButton.qml
  src/controls/templates/SwipeListItem.qml
  src/controls/templates/private/BackButton.qml
  src/controls/templates/private/ForwardButton.qml

To: hein, #frameworks, mart
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22762: Take in consideration flat() for the All Applications menu

[Eike Hein]

hein added a comment.


  You got a point, but there's a higher-level thing with the Kicker backend in 
terms of where it came from and where it's going:
  
  - Originally it was created specifically for the Kicker menu with the backend 
custom-designed to that UI. AppsModel is fairly generic (good), and the 
RootModel subclass exists almost solely to create the specific menu structure 
that the Kicker UI uses (it's the ugly wart in terms of this being a generic 
import, which it's increasingly used at)
  - Eventually we grew other UIs and RootModel keeps growing additional feature 
and mode knobs to customize it for them and it's getting rather sprawling
  
  I'd like to take the Kicker backend into a direction where it's more 
composable from QML to create different UIs rather than the equivalent of 
adding checkboxes to RootModel.
  
  In particular I think there's an opportunity to take a step towards that with 
this specific use case, which I'm guessing is to just get one flat list of 
apps, right?
  
  Namely I'd suggest:
  
  - AppsModel is already exposed to QML via the plugin and already implements 
QQmlParserStatus
  - Add a parameter-free constructor to AppsModel
  - Make entryPath a Q_PROPERTY to AppsModel and default it to empty (root 
level)
  
  Then whatever UI this is for can just do AppsModel{} and avoid all of the 
unnecessary stuff in RootModel and it's cleaner and a bit lighter.

REPOSITORY
  R119 Plasma Desktop

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

To: tcanabrava, davidedmundson, hein
Cc: hein, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22762: Take in consideration flat() for the All Applications menu

[Eike Hein]

hein added a comment.


  FYI, as maintainer of Kicker, I've reverted this commit in master until this 
review request is updated with additional info on what it's useful for. Please 
don't move patches through in <15 minutes without people having a chance to 
weigh in.

REPOSITORY
  R119 Plasma Desktop

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

To: tcanabrava, davidedmundson
Cc: hein, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D18860: Add comment / description to the translations kcm

[Eike Hein]

hein added a comment.


  Good stuff 

REPOSITORY
  R120 Plasma Workspace

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

To: jbbgameich, hein, #plasma, ngraham, bshah
Cc: bshah, sitter, apol, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, mart

D22738: Remove some uses on QDesktopWidget

[Eike Hein]

hein added inline comments.

INLINE COMMENTS

> pagermodel.cpp:123
> +};
> +for (QScreen* screen : qGuiApp->screens())
> +configureScreen(screen);

Missing braces around single-line block and no const

REPOSITORY
  R119 Plasma Desktop

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

To: apol, #plasma, davidedmundson
Cc: hein, davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22535: Make kimpanel a system tray applet

[Eike Hein]

hein added a subscriber: gpark.
hein added a comment.


  To catch this up to the GSoC status:
  
  
https://medium.com/@mujjingun_23509/google-summer-of-code-porting-keyboard-kcm-to-qt-quick-part-2-30c115771722
  
https://medium.com/@mujjingun_23509/google-summer-of-code-porting-keyboard-kcm-to-qt-quick-part-2-30c115771722
  
  Repo:
  
  
https://cgit.kde.org/clones/plasma-desktop/gpark/gsoc-kcm-keyboard-revamp.git/?h=new_keyboard_kcm
  
  There were nice videos showing the improved kimpanel and the switching w/ OSD 
and KCM open and everything being in sync somewhere, too, but I can't find them 
off-hand.
  
  To be clear: I'm not saying this patch is bad, but rather that we're in the 
fortunate position that it doesn't need to be pushed into slaying the 
configuration beast all alone as the above response did. It's not correct, and 
also not necessary because there's an existing body of work to leverage!
  
  Between @gpark, @guoyunhe, me and others, the right thing to do is form a 
team to advance this together, and make this patch part of it. A good place to 
organize this effort can be around T11054 .

REPOSITORY
  R119 Plasma Desktop

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

To: guoyunhe, ngraham, #vdg, #plasma
Cc: gpark, hein, mart, ngraham, apol, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas

D22535: Make kimpanel a system tray applet

[Eike Hein]

hein added subscribers: mart, hein.
hein added a comment.


  The configuration should be in System Settings, not in this applet, as 
they're system-global and override keyboard layout settings.
  
  The 2018 GSoC work started to implement this and got far, and also made 
kimpanel replace the existing tray keyboard layout indicator in the process and 
update the OSD and all the other system parts to understand input methods and 
switch between then. We should look into leveraging at least the applet changes 
right now and merging them with this patch.
  
  As for the conceptual change: It's a change I also suggested once, but was 
veto'd by the kimpanel maintainer at the time who was strongly of the opinion 
that every icon needs to be visible at all times. The discussion between him, 
me and @mart at the time was to spawn multiple SNIs and teach the tray to sort 
them together. I think this is workable but still advocate a single icon by 
default unless others are explicitly enabled.

REPOSITORY
  R119 Plasma Desktop

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

To: guoyunhe, ngraham, #vdg, #plasma
Cc: hein, mart, ngraham, apol, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas

D22306: Do not skip code launching application in application dashboard

[Eike Hein]

hein added a comment.


  Thanks, this looks a lot better and cleaner. Minor nitpick to clean up.

INLINE COMMENTS

> ItemGridView.qml:440
> +itemGrid.itemActivated(pressedItem.itemIndex, "", null);
> +} else if (!dragHelper.dragging && !pressedItem && 
> mouse.button == Qt.LeftButton)
> +root.toggle();

Coding style: Even for single-line blocks we require braces.

REPOSITORY
  R119 Plasma Desktop

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

To: luc4, ngraham, trmdi, #plasma
Cc: hein, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D12916: Panel should not stop auto-hiding if a window wants attention

[Eike Hein]

hein added a comment.


  In D12916#500367 , @davidedmundson 
wrote:
  
  > "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. 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.

REPOSITORY
  R119 Plasma Desktop

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

To: michaelmoon, ngraham, hein
Cc: hein, ngraham, davidedmundson, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21829: Improved Touch support for Kickoff

[Eike Hein]

hein added a comment.


  In D21829#498921 , @steffenh wrote:
  
  > I have the last days looking at plasma-frameworks, but my knowledge of the 
working of plasma is close to zero, so I am afraid I'm not able to do this.
  
  
  Before we discuss alternatives: Are you willing to have me try and teach you? 
I'm actually going to be very busy with a move in the next few weeks, so this 
could end up taking quite significant time between the two of us, but if you 
have the time and patience I don't mind helping you through creating the patch 
I had in mind.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff

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

To: steffenh, #plasma, hein, ngraham
Cc: mart, sbergeron, davidedmundson, ngraham, plasma-devel, LeGast00n, 
jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12916: Panel should not stop auto-hiding if a window wants attention

[Eike Hein]

hein added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

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

To: michaelmoon, ngraham, hein
Cc: hein, ngraham, davidedmundson, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12916: Panel should not stop auto-hiding if a window wants attention

[Eike Hein]

hein added a comment.


  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?

REPOSITORY
  R119 Plasma Desktop

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

To: michaelmoon, ngraham, hein
Cc: hein, ngraham, davidedmundson, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12916: Panel should not stop auto-hiding if a window wants attention

[Eike Hein]

hein added a comment.


  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)
  - It won't address other applets that may use the same 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.

REPOSITORY
  R119 Plasma Desktop

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

To: michaelmoon, ngraham
Cc: hein, ngraham, davidedmundson, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22306: Do not skip code launching application in application dashboard

[Eike Hein]

hein added a comment.


  First of, run `kdebugsettings` and make sure that debug output isn't disabled 
on your system.
  
  Then you can e.g. stop plasmashell from a terminal with: `kquitapp5 
plasmashell`
  
  And restart it with `plasmashell`, and you'll see `console.log` output on the 
terminal.
  
  If you don't want to muck with your plasmashell, you can also run a seperate 
instance of just Dashboard, which will also cut down on unrelated debug output 
noise: `plasmawindowed org.kde.plasma.kickerdash`

REPOSITORY
  R119 Plasma Desktop

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

To: luc4, ngraham, trmdi, #plasma
Cc: hein, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D18860: Add comment / description to the translations kcm

[Eike Hein]

hein added a comment.


  How about - and sorry if you're tired of the patch by now - "Change workspace 
and application languages"?

REPOSITORY
  R120 Plasma Workspace

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

To: jbbgameich, hein, #plasma, ngraham
Cc: bshah, sitter, apol, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, mart

D22506: Read BAMF_DESKTOP_FILE_HINT from environment

[Eike Hein]

hein added a comment.


  The idea and approach are good.
  
  But disk I/O in a UI hotpath is pretty scary. I do think a cache would be a 
good addition.
  
  In general I think libtm could use some improvements on its currently very 
coarse cache eviction scheme. It's mostly fine but there are some edge cases 
where we end up doing I/O on things we know we haven't changed. Unfortunately 
though improving that just means doing the legwork of going case-by-case and 
the complexity it invites ... i.e. a cache here.
  
  (Slightly unrelated: I have the beginnings of a patch set that uses 
KUserFeedback to gather telemetry on how often we use which identification 
means and why. I have this persistent feeling that we're falling through to the 
suboptimal/expensive code paths way more than we should currently, and there 
are probably minor changes to do "higher up in the file" to fix that. One is 
probably the neverending woes around normalizing for reverse DNS vs. plain 
.desktop file names. But in general I'd love for my system to be able go give 
me a trend curve for "Task Manager app identification expensiveness over time" 
to I can see when there's new regressions and/or changes in the app landscape.)

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, hein
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22503: kcms: translations: use availableDomainTranslations from plasmashell instead of systemsettings

[Eike Hein]

hein added a comment.


  Sorry, the cryptic comment can be safely ignored. I had a concern at first, 
then looked into it, decided I'm OK with the patch and forgot the started 
sentence. :-)

REPOSITORY
  R120 Plasma Workspace

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

To: jbbgameich, hein
Cc: bshah, hein, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D22503: kcms: translations: use availableDomainTranslations from plasmashell instead of systemsettings

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  This creayes

REPOSITORY
  R120 Plasma Workspace

BRANCH
  translations-no-systemsettings (branched from master)

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

To: jbbgameich, hein
Cc: bshah, hein, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D22306: Do not skip code launching application in application dashboard

[Eike Hein]

hein added a comment.


  I see what this is trying to do, but some of the details seem a bit wrong.
  
  E.g. you're only setting `dragging` to false in a code branch that's only 
executed when it already is false, which means it's not going to be set to 
false on a drag release, and pressX/Y also won't be unset on a release that 
doesn't happen above an iten anymore. You can keep the code flow change (the 
main thing this is probably fixing is that wrong check for the return value of 
`updatePositionProperties`), but please rework the patch a bit to make sure 
that the state of these variables still gets updated in the way they should on 
a release.

REPOSITORY
  R119 Plasma Desktop

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

To: luc4, ngraham, trmdi, #plasma
Cc: hein, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D18860: Add comment / description to the translations kcm

[Eike Hein]

hein added a comment.


  Agreed with Harald, the wording's misleading sorry :)

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

To: jbbgameich, hein, #plasma, ngraham
Cc: bshah, sitter, apol, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, mart

Re: how about a developer KCM

[Eike Hein]

On 7/16/19 11:36 PM, Nate Graham wrote:

On 7/16/19 8:32 AM, David Edmundson wrote:

However, I fear it will end up becoming some sort of hidden
plasma-tweaks, where we dump general aesthetic/behaviour settings in
here which we think are too niche for the main UI.
I don't want that to happen.


Strongly agreed.


Same.

I think DrKonqi-related options are natural fit in 'kdebugsettings' 
however. This app already exists, is essentially for devs, and has 
domain overlap with what DrKonqi does.





Nate


Cheers,
Eike

D22230: [TaskManager] Follow KDE title & subtitle style

[Eike Hein]

hein added a comment.


  @filipf: Yep, sounds sensible :)

REPOSITORY
  R119 Plasma Desktop

BRANCH
  tooltip-media-titlesubtitle-style (branched from master)

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

To: filipf, #plasma, #vdg, ngraham, hein
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22162: taskmanager: performance

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  In D22162#488378 , @ngraham wrote:
  
  > Not sure if it's related, but while you're at it, would you be able to also 
fix these errors when hovering over a pinned item:
  >
  >   
file:///home/nate/kde/usr/share/plasma/plasmoids/org.kde.plasma.taskmanager/contents/ui/Task.qml:347:
 Unable to assign [undefined] to int
  >   
file:///home/nate/kde/usr/share/plasma/plasmoids/org.kde.plasma.taskmanager/contents/ui/Task.qml:375:
 TypeError: Type error
  >
  
  
  They're not related, so shouldn't be handled in this review (see also fregl's 
patches).

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: apol, #plasma, hein
Cc: hein, ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D22053: [libtaskmanager] Track stacking order and window activation (on X11)

[Eike Hein]

hein created this revision.
hein added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
hein requested review of this revision.

REVISION SUMMARY
  `TaskGroupingProxyModel::requestToggleMaximized` now uses this to
  minimize and restore groups of windows while preserving the
  stacking order, a frequently user-requested wish.
  BUG:368867
  
  Window activation is additionally tracked to implement a new front-
  end feature to activate the most recently active window (or fall
  through to stacking order otherwise) subsequently.
  CCBUG:379259
  
  A Wayland implementation requires the addition of a
  `PlasmaWindowManagement::stackingOrder()`, which should be a QList
  of PlasmaWindow* in stacking order, along with a change signal. We
  discussed this at the Plasma+KWin sprint and I'll code up patches
  to KWin and KWayland soon and then implement the new API in here.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  libtaskmanager/abstracttasksmodel.h
  libtaskmanager/taskgroupingproxymodel.cpp
  libtaskmanager/xwindowtasksmodel.cpp

To: hein, #plasma
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D21983: [Kicker/Kickoff/Dash] Show Calculator and Unit Converter runners

[Eike Hein]

hein added a comment.


  You can do it for Dash, though -- check that `isDash` check where it already 
adds some runners to the Dash it doesn't add to App Menu.
  
  I'm fine with doing it for Kicker later, but we need to resolve the design 
issue first so we don't create a new bug.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-more-runners (branched from Plasma/5.16)

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

To: ngraham, #vdg, #plasma, hein, filipf
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21983: [Kicker/Kickoff/Dash] Show Calculator and Unit Converter runners

[Eike Hein]

hein added a comment.


  Let's only do it for Kickoff and the Dash? The Kicker UI is really not made 
for arbitrary runners.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-more-runners (branched from Plasma/5.16)

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

To: ngraham, #vdg, #plasma, hein, filipf
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22004: Increment iterator before any potential continue

[Eike Hein]

hein added a comment.


  I'm not super happy with hardcoding directory separators and schemas, 
judicious use of `QUrl::fromLocalFile` or `QUrl::fromUserInput` would probably 
be better to norm behavior.

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, hein
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21990: Don't propagate a title change if it didn't actually change

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  Ironically quite similar to a DDoS vector on the Task Manager. Apps suck :)

REPOSITORY
  R856 Plasma Browser Integration

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

To: broulik, #plasma, fvogt, hein
Cc: hein, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D21988: Reject duplicate events with identical coordinates

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:0628dab8f42b: Reject duplicate events with identical 
coordinates (authored by hein).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21988?vs=60316=60319

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml

To: hein, #plasma, ngraham
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D21978: RFC: Fix search not returning results in Kickoff when cursor is below tabbar

[Eike Hein]

hein added a comment.


  In D21978#483972 , @hein wrote:
  
  > That's wrong, sorry - I'm uploading a different patch that fixes the 
underlying cause.
  
  
  D21988 

REPOSITORY
  R119 Plasma Desktop

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

To: filipf, hein, #plasma, ngraham
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21988: Reject duplicate events with identical coordinates

[Eike Hein]

hein created this revision.
hein added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
hein requested review of this revision.

REVISION SUMMARY
  BUG:401861

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/FullRepresentation.qml

To: hein, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21978: RFC: Fix search not returning results in Kickoff when cursor is below tabbar

[Eike Hein]

hein added a comment.


  That's wrong, sorry - I'm uploading a different patch that fixes the 
underlying cause.

REPOSITORY
  R119 Plasma Desktop

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

To: filipf, hein, #plasma, ngraham
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21829: Improved Touch support for Kickoff

[Eike Hein]

hein added a subscriber: mart.
hein added a comment.


  I'm in principle quite OK with this minus that other patch I need to be 
written first to move DragHelper out into the lib (see other comment). Pitch: 
If you do that, it'll make it much easier to make the other menus (and other 
things) act similarly later, with smaller patches there.
  
  However we need a collective decision that this way to handle tap-and-hold is 
the one we want to go forward with - this patch basically makes a call to 
resolve T10783  in a particular way - and 
needs some more buy-in e.g. from @mart for that reason.

INLINE COMMENTS

> draghelper.cpp:91
>  drag->setPixmap(icon.pixmap(m_dragIconSize, m_dragIconSize));
> +if (touch) {
> +drag->setHotSpot(QPoint(m_dragIconSize*0.5,m_dragIconSize*0.95));

I'm not happy with hardcoding these values. If we decide to do touch drag this 
way, we're going to see this being duplicated all around the codebase - with 
subtly different values, etc. If we want it that way we should centralize it 
somewhere.

We probably need to put DragHelper to plasma-framework - there's similar 
classes in FV and TM too we can consolidate. This isn't that much work and I'll 
gladly help mentoring the patch.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff

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

To: steffenh, #plasma, hein, ngraham
Cc: mart, sbergeron, davidedmundson, ngraham, plasma-devel, LeGast00n, 
jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D21954: Handle Gimp 2.10

[Eike Hein]

hein added a comment.


  In D21954#483133 , @ngraham wrote:
  
  > Could this go to the stable branch?
  
  
  Aye, backported.

REPOSITORY
  R120 Plasma Workspace

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

To: hein, ngraham
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D21954: Handle Gimp 2.10

[Eike Hein]

hein added a comment.


  In D21954#483114 , @ngraham wrote:
  
  > Can we also pre-emptively add 2.12 as well? :)
  
  
  Done. :)

REPOSITORY
  R120 Plasma Workspace

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

To: hein, ngraham
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D21954: Handle Gimp 2.10

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:96655354961d: Handle Gimp 2.10 (authored by hein).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D21954?vs=60200=60203#toc

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21954?vs=60200=60203

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

AFFECTED FILES
  libtaskmanager/taskmanagerrulesrc

To: hein, ngraham
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D21954: Handle Gimp 2.10

[Eike Hein]

hein created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
hein requested review of this revision.

REVISION SUMMARY
  GIMP 2.10 is still being a bad desktop citizen and requires a workaround
  of this sort as well.
  
  A nicer fix to do at a later time would be a regex-based rewriting rule
  that won't need version updates.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  libtaskmanager/taskmanagerrulesrc

To: hein
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

Fwd: XDC 2019: Less than three weeks to go to submit your talks, workshops or demos!

[Eike Hein]

 Forwarded Message 
Subject: XDC 2019: Less than three weeks to go to submit your talks, 
workshops or demos!

Date: Thu, 20 Jun 2019 11:55:39 -0400
From: Mark Filion 
Organization: Collabora Ltd.
To: eve...@lists.x.org

Hello!

Less than three weeks to go to submit your talks, workshops or demos
for this year's X.Org Developer Conference, which will be taking place
in Montréal, Canada on October 2-4, 2019!

The 2019 X.Org Developers Conference is the annual technical meeting
for X Window System and Free Desktop developers. Attendees will gather
to discuss outstanding technical issues related to the Open Source
Graphics stack (Linux kernel, Mesa, DRM, Wayland, X11, etc.) and its
software ecosystem.

While any serious proposal will be gratefully considered, topics of
interest
to X.Org and freedesktop.org developers are encouraged. The program
focus is on new development, ongoing challenges and anything else that
will spark discussions among attendees in the hallway track.

We are open to talks across all layers of the graphics stack, from the
kernel
to desktop environments / graphical applications and about how to make
things better for the developers who build them.
Head to the XDC website to learn more:
https://xdc2019.x.org/

The deadline for submissions Sunday, 7 July 2019.

Best,

Mark

___
wayland-devel mailing list
wayland-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

D21803: PanelView: align setting of masks with how it's done for dialogs/tooltips

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  Thanks for restoring my sanity & faith in panels, this annoyed me for weeks :)

REPOSITORY
  R120 Plasma Workspace

BRANCH
  alignpanelmaskhandlingtodialog

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

To: kossebau, #plasma, hein
Cc: hein, plasma-devel, LeGast00n, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20237: Port to new KWorkspace API

[Eike Hein]

hein updated this revision to Diff 59660.
hein added a comment.


  - Fix typo
  - Rebase

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20237?vs=55487=59660

BRANCH
  master

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

AFFECTED FILES
  applets/kicker/CMakeLists.txt
  applets/kicker/package/contents/ui/ItemGridDelegate.qml
  applets/kicker/package/contents/ui/ItemListDelegate.qml
  applets/kicker/plugin/abstractmodel.cpp
  applets/kicker/plugin/actionlist.h
  applets/kicker/plugin/systementry.cpp
  applets/kicker/plugin/systementry.h
  applets/kicker/plugin/systemmodel.cpp
  applets/kicker/plugin/systemmodel.h
  applets/kickoff/package/contents/ui/KickoffItem.qml

To: hein, #plasma, davidedmundson
Cc: broulik, apol, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, mart

D19829: kicker: add option to make root level icons visible

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  I'm still a bit grumpy about this and dragging my feet! My gut still says 
it's both option clutter and UI noise :-)
  
  However:
  
  - There's a fair amount of people calling for this now.
  - The underlying code is already there.
  - Since the codepath is already in, it's more likely to bitrot when there 
isn't a GUI option that allows testing it.
  
  It's a go.

REPOSITORY
  R119 Plasma Desktop

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

To: i.Dark_Templar, hein, #plasma, #vdg, GB_2
Cc: GB_2, ngraham, plasma-devel, jraleigh, ragreen, Pitel, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D21429: [Fonts KCM] Switch default UI font to Noto Sans Display

[Eike Hein]

hein added a comment.


  I'm not sold on this. "Display" fonts typically have spacing and metrics 
optimized for large sizes and use cases like signage. The FAQ entry quoted in 
the task you linked talks about Noto Sans UI. I'd like some more solid 
documentation on how Display relates to this "UI" cut or something from 
upstream.

REPOSITORY
  R119 Plasma Desktop

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

To: GB_2, #plasma, #vdg, bshah
Cc: hein, bshah, plasma-devel, #vdg, #plasma, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21429: [Fonts KCM] Switch default UI font to Noto Sans Display

[Eike Hein]

hein added a comment.


  Why?

REPOSITORY
  R119 Plasma Desktop

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

To: GB_2, #plasma, #vdg, bshah
Cc: hein, bshah, plasma-devel, #vdg, #plasma, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21336: Get PaginateModel class ready for frameworks

[Eike Hein]

hein added a comment.


  I wouldn't call this implementation ready for frameworks, though - it just 
resets all the time, which is very slow as it forces views to destroy and 
recreate delegates instead of just updating them. A better implementation would 
just (re)filter source rows corresponding to the current page. This could be 
done as a subclass of `QSortFilterProxyModel` or custom; KItemModels' 
`KRearrangeColumnsProxyModel` can serve as inspiration to some degree. As it 
stands I wouldn't uactually use this if it was in the framework.

REPOSITORY
  R134 Discover Software Store

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

To: apol, davidedmundson
Cc: hein, davidedmundson, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D20265: Introduce libnotificationmanager

[Eike Hein]

hein added inline comments.

INLINE COMMENTS

> job.cpp:30
> +
> +#include 
> +

Unused?

> notificationgroupcollapsingproxymodel.cpp:194
> +{
> +if (source_parent.isValid() && m_limit > 0) {
> +if (!m_expandedGroups.isEmpty() && 
> m_expandedGroups.contains(source_parent)) {

Check m_limit first for a tiny speedup.

> notifications.cpp:76
> +static uint notificationId(const QModelIndex );
> +QModelIndex mapFromModel(const QModelIndex ) const;
> +

Have you considered using `KModelIndexProxyMapper` from KF5::ItemModels?

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: hein, mart, nicolasfella, davidedmundson, ngraham, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D20265: Introduce libnotificationmanager

[Eike Hein]

hein added inline comments.

INLINE COMMENTS

> notificationgroupcollapsingproxymodel.cpp:175
> +emit dataChanged(idx, idx, dirtyRoles);
> +emit dataChanged(idx.child(0, 0), idx.child(rowCount(idx) - 1, 0), 
> dirtyRoles);
> +

I know this really sucks, and I'm not going to be pushy on this because you've 
been waiting on this review forever.

But: Qt 5.13 has deprecated `QModelIndex::child` (they want us to use 
`QAIM::index` now always), leading to noisy builds. For new code it'd be 
amazing to avoid it.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: hein, mart, nicolasfella, davidedmundson, ngraham, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D20265: Introduce libnotificationmanager

[Eike Hein]

hein added inline comments.

INLINE COMMENTS

> notificationgroupcollapsingproxymodel.cpp:52
> +Q_UNUSED(bottomRight); // what about it?
> +Q_UNUSED(roles);
> +

Not actually unused.

`bottomRight`: You sure the source model will emit for single cells? Otherwise 
you need to loop over the range.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: hein, mart, nicolasfella, davidedmundson, ngraham, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D20265: Introduce libnotificationmanager

[Eike Hein]

hein added inline comments.

INLINE COMMENTS

> notificationgroupingproxymodel.cpp:38
> +
> +bool NotificationGroupingProxyModel::appsMatch(const QModelIndex , const 
> QModelIndex ) const
> +{

Careful there - these will match when both are empty, which is why the libtm 
version tests for that. Are you sure this data cannot ever be missing?

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: hein, mart, nicolasfella, davidedmundson, ngraham, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D20265: Introduce libnotificationmanager

[Eike Hein]

hein added inline comments.

INLINE COMMENTS

> notificationsmodel.cpp:57
> +
> +int rowOfNotification(uint id) const;
> +

Could be worth making `inline` maybe?

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: hein, mart, nicolasfella, davidedmundson, ngraham, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D20265: Introduce libnotificationmanager

[Eike Hein]

hein added inline comments.

INLINE COMMENTS

> notifications.cpp:828
> +{
> +return QHash {
> +{IdRole, QByteArrayLiteral("notificationId")}, // id is QML-reserved

While this isn't an objection, I usually use QMetaEnum to compute this from the 
enum instead of having a block of stuff that needs to be synced.

> notifications.h:211
> +
> +enum Roles {
> +IdRole = Qt::UserRole + 1, ///< A notification identifier. This can 
> be uint notification ID or string application job source.

I recommend making this Q_ENUM and registering it; with libtm it turned out 
that it's useful to be able to call data() with a role from QML sometimes.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: hein, mart, nicolasfella, davidedmundson, ngraham, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol