Differential D22827

Scrollview - Don't fill the parent with the view
ClosedPublic
Actions

Authored by davidre on Jul 30 2019, 12:51 PM.

Details

Reviewers
mart
ngraham
Commits
R296:e310144eb8e9: Scrollview - Don't fill the parent with the view
Summary

If the style sets a padding this is not respected if we set anchors.fill: parent.

BUG: 407643

Test Plan

Before:


After:

Diff Detail

Repository
R296 KDeclarative
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidre created this revision.Jul 30 2019, 12:51 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJul 30 2019, 12:51 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
davidre requested review of this revision.Jul 30 2019, 12:51 PM
davidre updated this revision to Diff 62791.Jul 30 2019, 12:52 PM

Commit message

davidre retitled this revision from Scrollview - Don't fill the parent with the view If the style sets a padding this is not respected if we set `anchors.fill: parent`. to Scrollview - Don't fill the parent with the view.Jul 30 2019, 12:55 PM
davidre edited the summary of this revision. (Show Details)
davidre edited the test plan for this revision. (Show Details)
davidre added a reviewer: mart.
davidre edited the test plan for this revision. (Show Details)
davidre added a comment.Jul 30 2019, 12:57 PM

An alternative approach could be to set the respective anchor.*margin to view.parent.*padding.

filipf added a subscriber: filipf.Jul 30 2019, 3:41 PM

Also fixes the Virtual Desktops KCM fwiw

davidre edited the summary of this revision. (Show Details)Jul 30 2019, 6:29 PM
davidre added a comment.Aug 1 2019, 8:36 AM

So the alternative approach would be something like this:

diff --git a/src/qmlcontrols/kcmcontrols/qml/ScrollView.qml b/src/qmlcontrols/kcmcontrols/qml/ScrollView.qml
index 8ef57a2..443f063 100644
--- a/src/qmlcontrols/kcmcontrols/qml/ScrollView.qml
+++ b/src/qmlcontrols/kcmcontrols/qml/ScrollView.qml
@@ -48,14 +48,28 @@ QtControls.ScrollView {
     onViewChanged: {
         view.parent = scroll;
         view.anchors.fill = view.parent;
+        setMargins();
     }
 
     activeFocusOnTab: false
     Kirigami.Theme.colorSet: Kirigami.Theme.View
     Kirigami.Theme.inherit: false
 
-    Component.onCompleted: scroll.background.visible = true;
+    Component.onCompleted: {
+        scroll.background.visible = true;
+        setMargins();
+    }
+
+    function setMargins() {
+        if (view.parent.padding) {
+            view.anchors.padding = view.parent.padding;
+        } else {
+            view.anchors.topMargin = view.parent.topPadding;
+            view.anchors.leftMargin = view.parent.leftPadding;
+            view.anchors.bottomMargin = view.parent.bottomPadding;
+            view.anchors.rightMargin = view.parent.rightPadding;
+        }
+    }
 
-    
     QtControls.ScrollBar.horizontal.visible: false
 }

I has the same result but maybe would be more correct?

ngraham accepted this revision.Aug 1 2019, 5:12 PM
ngraham added a subscriber: ngraham.

This looks like a sane and simpler implementation to me, but let's get @mart's opinion too.

This revision is now accepted and ready to land.Aug 1 2019, 5:12 PM
mart accepted this revision.Aug 1 2019, 5:18 PM
Closed by commit R296:e310144eb8e9: Scrollview - Don't fill the parent with the view (authored by davidre). · Explain WhyAug 1 2019, 5:35 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 62917

View Options

src/qmlcontrols/kcmcontrols/qml/ScrollView.qml

Loading...