[Application Style] Add GTK Application Style Page
ClosedPublic

Authored by gikari on Sat, Jan 11, 1:34 PM.

Details

Summary

The GTK KCM is now moved to Application Style KCM as a sub page. The
functionality is identical to the one, found in GTK KCM, as well for the
bugs :)

The page is only accessible, if the gtkconfig kded module is loaded,
because only with the module the configuration can be changed.

Depends on D26261

Test Plan
  1. Apply D26261
  2. Restart kded5
  3. Open System Settings, go to Application Style KCM
  4. The button for GTK Configuration should be present
  5. Go to Startup and Shutdown > Background Services, disable Plasma GTKd
  6. Return to the App Style KCM, the button should disappear
  7. Return to the Kded KCM and enable Plasma GTKd back
  8. Return to the App Style KCM - button is present now
  9. Go to subpage
  10. Happy (I hope) testing!

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.
gikari created this revision.Sat, Jan 11, 1:34 PM
Restricted Application added a project: Plasma. · View Herald TranscriptSat, Jan 11, 1:34 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
gikari requested review of this revision.Sat, Jan 11, 1:34 PM
gikari edited the test plan for this revision. (Show Details)Sat, Jan 11, 1:38 PM
gikari added a reviewer: VDG.
gikari updated this revision to Diff 73272.Sat, Jan 11, 1:48 PM
gikari edited the test plan for this revision. (Show Details)

Do not init GtkPage*, if the kded module is not loaded

gikari updated this revision to Diff 73313.Sat, Jan 11, 10:59 PM

Remove unnessary include and containsTheme function

gikari updated this revision to Diff 73390.Mon, Jan 13, 11:40 AM

Replace include gtkpage with forward declaration

gikari updated this revision to Diff 73425.Mon, Jan 13, 3:28 PM

Undo previous hasty decision

ngraham accepted this revision.Mon, Jan 13, 3:31 PM
ngraham added a subscriber: ngraham.

Visually and behaviorally, this is great. Probably needs a Plasma review too.

This revision is now accepted and ready to land.Mon, Jan 13, 3:31 PM
gikari updated this revision to Diff 73429.Mon, Jan 13, 3:40 PM

Replace themeUpdate redundant signal with direct load call.

gikari updated this revision to Diff 73519.Tue, Jan 14, 12:56 PM

Remove Q_SLOT keyword from load and add override to destructor

davidedmundson added inline comments.
kcms/style/gtkthemesmodel.cpp
108

this needs to be

if (parent.isValid()) {
return m_themesList.count();
}
return 0;

Otherwise your're saying every item has children with N rows, which have children with N rows forever.

gikari added inline comments.Tue, Jan 14, 3:07 PM
kcms/style/gtkthemesmodel.cpp
108

Did you mean

if (parent.isValid()) {
    return 0;
}
return m_themesList.count();

?

broulik added inline comments.Tue, Jan 14, 3:07 PM
kcms/style/gtkthemesmodel.cpp
108

I think so :D

gikari updated this revision to Diff 73534.Tue, Jan 14, 3:13 PM
gikari marked 3 inline comments as done.

Remove columnCount method and fix rowCount method

gikari updated this revision to Diff 73536.Tue, Jan 14, 3:20 PM

Return accidentally removed empty lines in kcmstyle.h

mart accepted this revision.Wed, Jan 15, 3:11 PM
mart accepted this revision.
This revision was automatically updated to reflect the committed changes.
zzag added a subscriber: zzag.Wed, Jan 15, 3:14 PM
zzag added inline comments.
kcms/style/gtkthemesmodel.h
37

Whar's the purpose of this destructor?

gikari added inline comments.Wed, Jan 15, 3:24 PM
kcms/style/gtkthemesmodel.h
37

No purpose, therefore can I just delete it?

zzag added inline comments.Wed, Jan 15, 3:42 PM
kcms/style/gtkthemesmodel.h
37

Yes, I don't see why one would want ~Class() override = default in a header file.