D10502: Eliminate unnecessary bottom pading on OverlaySheets

2018-02-14 Thread Aleix Pol Gonzalez 
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

2018-02-14 Thread Nathaniel Graham 
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

2018-02-14 Thread Nathaniel Graham 
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

2018-02-14 Thread Nathaniel Graham 
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=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

2018-02-14 Thread Aleix Pol Gonzalez 
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

2018-02-13 Thread Nathaniel Graham 
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

2018-02-13 Thread Nathaniel Graham 
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