D10502: Eliminate unnecessary bottom pading on OverlaySheets
apol added a comment. +1 makes sense to me. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D10502 To: ngraham, #kirigami, apol Cc: plasma-devel, apol, davidedmundson, mart, hein
D10502: Eliminate unnecessary bottom pading on OverlaySheets
ngraham edited the summary of this revision. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D10502 To: ngraham, #kirigami, apol Cc: plasma-devel, apol, davidedmundson, mart, hein
D10502: Eliminate unnecessary bottom pading on OverlaySheets
ngraham marked an inline comment as done. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D10502 To: ngraham, #kirigami, apol Cc: plasma-devel, apol, davidedmundson, mart, hein
D10502: Eliminate unnecessary bottom pading on OverlaySheets
ngraham updated this revision to Diff 27150. ngraham added a comment. Use a more programmatically correct approach (top_adding+buttomPadding instead of Units.gridUnit*2); this will work for clients that change the padding values REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10502?vs=27133&id=27150 BRANCH less-popup-padding REVISION DETAIL https://phabricator.kde.org/D10502 AFFECTED FILES src/controls/templates/OverlaySheet.qml To: ngraham, #kirigami, apol Cc: plasma-devel, apol, davidedmundson, mart, hein
D10502: Eliminate unnecessary bottom pading on OverlaySheets
apol added inline comments. INLINE COMMENTS > OverlaySheet.qml:270 > width: root.contentItem.implicitWidth <= 0 ? mainItem.width : > Math.max(mainItem.width/2, Math.min(mainItem.width, > root.contentItem.implicitWidth)) > -height: scrollView.flickableItem && > root.contentItem.hasOwnProperty("contentY") ? > scrollView.flickableItem.contentHeight + headerHeight : > (root.contentItem.height + topPadding + bottomPadding + > Units.iconSizes.medium + Units.gridUnit) > +height: scrollView.flickableItem && > root.contentItem.hasOwnProperty("contentY") ? > scrollView.flickableItem.contentHeight + headerHeight : > (root.contentItem.height + Units.gridUnit * 2) > Item { Does this mean that setting the topPadding and bottomPadding to 0 would make this empty space go away? REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D10502 To: ngraham, #kirigami, apol Cc: plasma-devel, apol, davidedmundson, mart, hein
D10502: Eliminate unnecessary bottom pading on OverlaySheets
ngraham edited the test plan for this revision. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D10502 To: ngraham, #kirigami, apol Cc: plasma-devel, apol, davidedmundson, mart, hein
D10502: Eliminate unnecessary bottom pading on OverlaySheets
ngraham created this revision. ngraham added reviewers: Kirigami, apol. Restricted Application added a project: Kirigami. Restricted Application added a subscriber: plasma-devel. ngraham requested review of this revision. REVISION SUMMARY The OverlaySheet already defines a bottomMargin, so we don't need to add so much extra space. BUG: 390032 TEST PLAN Tested with Discover's review input sheet. Before: After: Also tested in Kirigami Gallery; all pop-ups I could find still looked good. REPOSITORY R169 Kirigami BRANCH less-popup-padding REVISION DETAIL https://phabricator.kde.org/D10502 AFFECTED FILES src/controls/templates/OverlaySheet.qml To: ngraham, #kirigami, apol Cc: plasma-devel, apol, davidedmundson, mart, hein