[kcmkwin] Split out Desktop Effects KCM
ClosedPublic

Authored by zzag on Feb 3 2019, 5:03 PM.

Details

Summary

A while ago desktop effects and compositing settings lived under the
same roof

but time has passed and now those two have their own kcms. This causes
some issues:

  • for newcomers it's harder to find code of the Desktop Effects KCM;
  • git history doesn't look good, e.g. "[kcmkwin/compositing] Add some bugs to Desktop Effects KCM to fix later";
  • in general, the mix of two doesn't look good in the code.

This change splits out the Desktop Effects KCM. Unfortunately, in order
to have more nicer code I had to refactor EffectModel a little bit.

Before:

After:

Diff Detail

Repository
R108 KWin
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7843
Build 7861: arc lint + arc unit
zzag created this revision.Feb 3 2019, 5:03 PM
Restricted Application added a project: KWin. · View Herald TranscriptFeb 3 2019, 5:03 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Feb 3 2019, 5:03 PM
zzag updated this revision to Diff 50789.Feb 3 2019, 5:10 PM

update

Why is the title smaller now?

zzag added a comment.Feb 3 2019, 11:19 PM

Why is the title smaller now?

Probably because of KQuickAddons::ConfigModule. Virtual Desktops KCM also has smaller title.

ngraham added a subscriber: mart.Feb 3 2019, 11:53 PM
In D18703#404672, @zzag wrote:

Why is the title smaller now?

Probably because of KQuickAddons::ConfigModule. Virtual Desktops KCM also has smaller title.

@mart is this another case where we need to make the title use the same metrics as Kirigami's Heading?

zzag planned changes to this revision.Feb 4 2019, 1:24 AM
zzag updated this revision to Diff 50821.Feb 4 2019, 9:32 AM

Address coding style issues.

zzag updated this revision to Diff 50822.Feb 4 2019, 9:37 AM

Yet another sneaky update.

zzag updated this revision to Diff 50886.Feb 4 2019, 9:13 PM
  • Rebase on master; fix merge conflicts;
  • Change the title to "Configure Desktop Effects."
zzag edited the summary of this revision. (Show Details)Feb 4 2019, 9:18 PM

@ltoscano
We're splitting a .pot file into two .pots. Is there anything we can do to make translators lives easier?

kcmkwin/kwineffects/package/contents/ui/Effect.qml
51

Can you explain why we have _toggled and both onToggled and onClicked handlers

@ltoscano
We're splitting a .pot file into two .pots. Is there anything we can do to make translators lives easier?

We can copy create a copy of the existing translation file using the new name.
Will you keep the old .pot and add a new one with a subset of strings?

Will you keep the old .pot and add a new one with a subset of strings?

Yes.

old pot is kcmkwincompositing.pot
new pots will be kcmkwincompositing.pot and kcm_kwin_effects.pot

zzag added inline comments.Feb 21 2019, 3:01 PM
kcmkwin/kwineffects/package/contents/ui/Effect.qml
51

It appears that it's not possible to uncheck a radio button in an exclusive group. One would need that, for example, to disable the zoom effect.

zzag updated this revision to Diff 52227.Feb 21 2019, 6:05 PM

Retrieve proper transient parent from context.

Thank you, David.

zzag updated this revision to Diff 52273.Feb 22 2019, 9:28 AM

go back to the previous version

As for your transients problem.

We open the configuration UI with the QQuickView as the transient. This is not a top level as it's embedded in the KCM.

In this case QtWayland does try to look for parents or transientParents of the window.
But as this QQuickView is an offscreen surface it doesn't even have a parent or transient parent. QtWayland has nothing to go on.

This comes up in multiple places, such as GHNS in every QtQuick KCM

The more interesting question isn't why wayland doen't work, but why x does work.

zzag added a comment.EditedFeb 25 2019, 12:18 PM

It works on X11 because dialogs become group transients (this is both a feature and a bug).

davidedmundson accepted this revision.Mar 17 2019, 9:41 PM

At least that transient problem is fully understood. We have to solve that somewhere else as it affects the other KCMs.
Can you file a bug report (or phab task or whatever) copying the messages above so it doesn't get lost, and then lets get this shipped.

This revision is now accepted and ready to land.Mar 17 2019, 9:41 PM
zzag added a comment.Mar 18 2019, 8:49 AM

Can you file a bug report (or phab task or whatever)

Against which product/component should I file a bug report?

This revision was automatically updated to reflect the committed changes.