Skip to content

Commit

Permalink
Cleanup - Don't count item padding in ExpandableLayout
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Mugurell authored and mergify[bot] committed Apr 14, 2021
1 parent b4e2368 commit 5b3677a
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -328,17 +329,14 @@ internal class ExpandableLayout private constructor(context: Context) : FrameLay
result += listView.paddingTop
result += listView.paddingBottom

run loop@ {
run loop@{
listView.children.forEachIndexed { index, view ->
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -617,10 +617,10 @@ 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
expected += itemHeightForEachProperty // height of the sticky item
assertEquals(expected, result)
}

Expand All @@ -646,9 +646,9 @@ 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 // height of the sticky item
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)
}
}
Expand Down

0 comments on commit 5b3677a

Please sign in to comment.