Commit 5b3677aa authored by Mugurell's avatar Mugurell Committed by mergify[bot]
Browse files

Cleanup - Don't count item padding in ExpandableLayout 

ExpandableLayout needs to iterate through all items until the one set as the
limit of the collapsible menu and accumulate the distance.
It adds the items height and margins but also erroneously the padding.

Stop counting the padding if height is counted. Height contains any vertical
padding set.
parent b4e2368f

components/browser/menu/src/main/java/mozilla/components/browser/menu/view/ExpandableLayout.kt

+2 −4
Original line number Diff line number Diff line
@@ -309,6 +309,7 @@ internal class ExpandableLayout private constructor(context: Context) : FrameLay 
    // That distance will be the collapsed height of the ViewGroup used when this will be first shown on the screen.

    // Users will be able to afterwards expand the ViewGroup to the full height.

    @VisibleForTesting

    @Suppress("ReturnCount")

    internal fun calculateCollapsedHeight(): Int {

        val listView = (wrappedView.getChildAt(0) as ViewGroup)

        // Simple sanity check
@@ -333,12 +334,9 @@ internal class ExpandableLayout private constructor(context: Context) : FrameLay 
                if (index < lastVisibleItemIndexWhenCollapsed) {

                    result += view.marginTop

                    result += view.marginBottom

                    result += view.paddingTop

                    result += view.paddingBottom

                    result += view.measuredHeight

                } else if (index == lastVisibleItemIndexWhenCollapsed) {

                    result += view.marginTop

                    result += view.paddingTop



                    // Edgecase: if the same item is the sticky footer and the lastVisibleItemIndexWhenCollapsed

                    // the menu will be collapsed to this item but shown with full height.

components/browser/menu/src/test/java/mozilla/components/browser/menu/view/ExpandableLayoutTest.kt

+8 −8
Original line number Diff line number Diff line
@@ -589,8 +589,8 @@ class ExpandableLayoutTest { 
        var expected = 0

        expected += viewHeightForEachProperty * 4 // marginTop + marginBottom + paddingTop + paddingBottom

        expected += listHeightForEachProperty * 4 // marginTop + marginBottom + paddingTop + paddingBottom

        expected += itemHeightForEachProperty * 5 // height + marginTop + marginBottom + paddingTop + paddingBottom for the top item shown in entirety

        expected += itemHeightForEachProperty * 2 // marginTop + paddingTop for the special view

        expected += itemHeightForEachProperty * 3 // height + marginTop + marginBottom for the top item shown in entirety

        expected += itemHeightForEachProperty // marginTop for the special view

        expected += itemHeightForEachProperty / 2 // as per the specs, show only half of the special view

        assertEquals(expected, result)

    }
@@ -617,8 +617,8 @@ class ExpandableLayoutTest { 
        var expected = 0

        expected += viewHeightForEachProperty * 4 // marginTop + marginBottom + paddingTop + paddingBottom

        expected += listHeightForEachProperty * 4 // marginTop + marginBottom + paddingTop + paddingBottom

        expected += itemHeightForEachProperty * 5 // height + marginTop + marginBottom + paddingTop + paddingBottom for the top item shown in entirety

        expected += itemHeightForEachProperty * 2 // marginTop + paddingTop for the special view

        expected += itemHeightForEachProperty * 3 // height + marginTop + marginBottom for the top item shown in entirety

        expected += itemHeightForEachProperty // marginTop for the special view

        expected += itemHeightForEachProperty / 2 // as per the specs, show only half of the special view

        expected += itemHeightForEachProperty // height of the sticky item

        assertEquals(expected, result)
@@ -646,8 +646,8 @@ class ExpandableLayoutTest { 
        var expected = 0

        expected += viewHeightForEachProperty * 4 // marginTop + marginBottom + paddingTop + paddingBottom

        expected += listHeightForEachProperty * 4 // marginTop + marginBottom + paddingTop + paddingBottom

        expected += itemHeightForEachProperty * 5 // height + marginTop + marginBottom + paddingTop + paddingBottom for the top item shown in entirety

        expected += itemHeightForEachProperty * 2 // marginTop + paddingTop for the special view

        expected += itemHeightForEachProperty * 3 // height + marginTop + marginBottom for the top item shown in entirety

        expected += itemHeightForEachProperty // marginTop for the special view

        expected += itemHeightForEachProperty // height of the sticky item

        assertEquals(expected, result)

    }