Unify KPageDialog margin handling into KPageDialog itself
ClosedPublic

Authored by ngraham on Nov 21 2019, 5:04 PM.

Details

Summary

This prevents us from having to manually specify the margins in all of the subclasses.

BUG: 413181
FIXED-IN: 5.65

Test Plan

Assistant dialog now has correct margins:

App settings window still has correct margins:

After applying D25451, Multi-KCM KCMShell dialog still has correct margins:

After applying D25451, single-KCM KCMShell dialog still has correct margins:

Please test extensively; this code seems somewhat fragile.

Diff Detail

Repository
R236 KWidgetsAddons
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Nov 21 2019, 5:04 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 21 2019, 5:04 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ngraham requested review of this revision.Nov 21 2019, 5:04 PM
kossebau added inline comments.
src/kassistantdialog.cpp
102 ↗(On Diff #70115)

Ideally q->pageWidget()->style() would be cached, given we are in hotpath here during startup, also helps codereading a bit.

ngraham updated this revision to Diff 70118.Nov 21 2019, 5:51 PM
ngraham marked an inline comment as done.

Cache the style

cfeck added a comment.Nov 21 2019, 8:01 PM

Since KAssistantDialog is a KPageDialog, what happens to other users of that class? I had hoped the fix is in KPageDialog, and it would only omit the margins when the caller requested it (e.g. KCMs etc.).

src/kassistantdialog.cpp
103 ↗(On Diff #70118)

Please also pass styleOptions and widget. Some styles might compute margins from font sizes, and without any context, they can only assume defaults.

ngraham planned changes to this revision.Nov 21 2019, 8:03 PM

Sure, I can look into that.

ngraham updated this revision to Diff 70128.Nov 21 2019, 9:59 PM

Do it universally

ngraham edited the test plan for this revision. (Show Details)Nov 21 2019, 10:05 PM
ngraham retitled this revision from Fix assistant dialog margins to Unify KPageDialog margin handling into KPageDialog itself.
ngraham edited the summary of this revision. (Show Details)
mart accepted this revision.Nov 29 2019, 2:14 PM
This revision is now accepted and ready to land.Nov 29 2019, 2:14 PM
This revision was automatically updated to reflect the committed changes.