D22083: introduce concept of header and footer for kpageview

mart added a comment. In D22083#498499 , @hpereiradacosta wrote: > Looking into kcmultidialog, it seems there is some internal handling of margins in there, for a reason unknown to me. Maybe you want to investigate there too (in a different

D22083: introduce concept of header and footer for kpageview

This revision was automatically updated to reflect the committed changes. Closed by commit R236:b581c5f990a6: introduce concept of header and footer for kpageview (authored by mart). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D22083?vs=61844=62554#toc REPOSITORY R236 KWidgetsAddons

D22083: introduce concept of header and footer for kpageview

ngraham added a task: T11279: Unify settings windows' sidebar appearance. REPOSITORY R236 KWidgetsAddons BRANCH mart/pageviewfooter REVISION DETAIL https://phabricator.kde.org/D22083 To: mart, #plasma, #frameworks, #vdg, ngraham Cc: hpereiradacosta, cfeck, ndavis, ngraham,

D22083: introduce concept of header and footer for kpageview

hpereiradacosta added a comment. Hi Marco, Patch works like a charm in standard config dialogs. As a somewhat off topic remark though, it does not work in kcmultidialog (in framework kcmutils), even though it derives from kpageddialog. This shows up for instance in the "window

D22083: introduce concept of header and footer for kpageview

mart added a comment. In D22083#496446 , @ngraham wrote: > Lovely. Looks perfect in Dolphin, Kate, Konsole, Okular, Gwenview, and Kile. > > Ark and Spectacle also need to be adapted, like kcmshell. Anything not using KPageView properly will

D22083: introduce concept of header and footer for kpageview

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Lovely. Looks perfect in Dolphin, Kate, Konsole, Okular, Gwenview, and Kile. Ark and Spectacle also need to be adapted, like kcmshell. Anything not using KPageView properly will need

D22083: introduce concept of header and footer for kpageview

mart added a comment. this makes it look almost pixel perfect F7031708: Screenshot_20190716_115035.png tough introduces a behavior change: moves the responsibility of adding margins from the outer layout to the internal one, making some users

D22083: introduce concept of header and footer for kpageview

mart updated this revision to Diff 61844. mart added a comment. - introduce concept of header and footer for kpageview - adress code style comments - use rowspan and colspan to add margins in the right places REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE

D22083: introduce concept of header and footer for kpageview

ngraham added a comment. Yep, I think so. I'd like to get the extra margins removed as well, but this is a good incremental step, and given how D22138 already landed, I think it's better to have this in. REPOSITORY R236 KWidgetsAddons REVISION

D22083: introduce concept of header and footer for kpageview

mart added a comment. can this part go forward? I think ui-wise looks fine even if the other parts won't make it for release? (still hope they will tough :) REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D22083 To: mart, #plasma, #frameworks, #vdg Cc:

D22083: introduce concept of header and footer for kpageview

ngraham added a task: T11153: Unify sidebar appearance. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D22083 To: mart, #plasma, #frameworks, #vdg Cc: hpereiradacosta, cfeck, ndavis, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns

D22083: introduce concept of header and footer for kpageview

hpereiradacosta added a comment. In D22083#491299 , @mart wrote: > code issues are fixed. > Now for removing the further margin around the whole window: > should that be done here or in breeze? > if done here, it may make it look bad on

D22083: introduce concept of header and footer for kpageview

mart added a comment. code issues are fixed. Now for removing the further margin around the whole window: should that be done here or in breeze? if done here, it may make it look bad on all other widget styles? REPOSITORY R236 KWidgetsAddons REVISION DETAIL

D22083: introduce concept of header and footer for kpageview

mart reopened this revision. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D22083 To: mart, #plasma, #frameworks, #vdg Cc: hpereiradacosta, cfeck, ndavis, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns

D22083: introduce concept of header and footer for kpageview

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. mart marked 3 inline comments as done. Closed by commit R236:fd5899d49d01: adress code style comments (authored by mart). CHANGED PRIOR TO

D22083: introduce concept of header and footer for kpageview

mart updated this revision to Diff 61206. mart added a comment. - introduce concept of header and footer for kpageview - adress code style comments REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22083?vs=60603=61206 BRANCH mart/pageviewfooter

D22083: introduce concept of header and footer for kpageview

hpereiradacosta added a comment. Hi Marco, see https://phabricator.kde.org/D22138 (I also included the white background in there, because it turned out a bit tricky). REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D22083 To: mart, #plasma, #frameworks,

D22083: introduce concept of header and footer for kpageview

mart added a comment. In D22083#487566 , @hpereiradacosta wrote: > ather simple and attached. Feel free to add this or something similar to any other modification you plan to do. F6938398: patch.diff >

D22083: introduce concept of header and footer for kpageview

hpereiradacosta added a comment. ather simple and attached. Feel free to add this or something similar to any other modification you plan to do. F6938398: patch.diff > Thank you very much Hugo.. that pixel was driving me mad ;) > Do you want to

D22083: introduce concept of header and footer for kpageview

mart added a comment. In D22083#487307 , @hpereiradacosta wrote: > On the breeze side, asside from the layout margins/spacing issues, there is at least one hard-coded pixel in the frame rendering that must be removed. In fact I have this

D22083: introduce concept of header and footer for kpageview

hpereiradacosta added a comment. On the breeze side, asside from the layout margins/spacing issues, there is at least one hard-coded pixel in the frame rendering that must be removed. In fact I have this committed in my local breeze version already. The corresponding patch is rather simple

D22083: introduce concept of header and footer for kpageview

cfeck added inline comments. INLINE COMMENTS > kpageview.cpp:458 > + > +if (d->pageHeader == header) { > +return; This check is duplicated. > kpageview.cpp:476 > +} > + > +} Please remove this empty line. > kpageview.h:163 > +/** > + * Set a widget as the header

D22083: introduce concept of header and footer for kpageview

ngraham added a subscriber: ndavis. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D22083 To: mart, #plasma, #frameworks, #vdg Cc: ndavis, ngraham, kde-frameworks-devel, LeGast00n, michaelh, bruns

D22083: introduce concept of header and footer for kpageview

ngraham accepted this revision as: VDG. ngraham added a comment. This seems to work quite well. For the remaining padding on the left, top, and bottom, does that come from Breeze, or would it need to be removed here? Or does it come from here and we should work around it in Breeze?

D22083: introduce concept of header and footer for kpageview

mart added a comment. white background and part of margins removal will have to probably be in breeze style F6931480: Screenshot_20190624_184812.png REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D22083 To: mart,

D22083: introduce concept of header and footer for kpageview

mart created this revision. mart added reviewers: Plasma, Frameworks, VDG. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. mart requested review of this revision. REVISION SUMMARY put the dialog buttons in the footer for kpagedialog this is the first step