ahiemstra added inline comments.

INLINE COMMENTS

> GlobalDrawer.qml:241
>  
> -    rightPadding: !Settings.isMobile && mainFlickable.contentHeight > 
> mainFlickable.height ? Units.gridUnit : Units.smallSpacing
> +    leftPadding: 0
> +    rightPadding: 0

+1 for getting rid of these paddings.

> GlobalDrawer.qml:252
> +            id: topContent
> +            spacing: 50
> +            Layout.alignment: Qt.AlignHCenter

Uhm, what's this value?

> GlobalDrawer.qml:260
> +            visible: children.length > 0 && childrenRect.height > 0 && 
> opacity > 0
> +            opacity: !root.collapsed || showTopContentWhenCollapsed
> +            Behavior on opacity {

In my opinion, it is nicer to be explicit about types instead of relying on 
implicit conversion. So this should be `(!root.collapsed || 
showTopContentWhenCollapsed) ? 1 : 0`.

> GlobalDrawer.qml:370
> +                        Layout.fillWidth: true
> +                        Layout.rightMargin: !Settings.isMobile && 
> mainFlickable.contentHeight > mainFlickable.height ? Units.gridUnit : 0
> +                        Layout.minimumHeight: currentItem ? 
> currentItem.implicitHeight : 0

We **really** should fix scrollview's scrollbars...

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D25019

To: ngraham, #kirigami, mart, apol, ahiemstra
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein

Reply via email to