[SnapHelper] Always respect padding when calculating the center of RecyclerView [139452422]

Fixed

Bug

Status Update

No update yet.

Description

je...@google.com

created issue #1

Aug 15, 2019 02:50PM

When PagerSnapHelper calculates the distance between the center of a View and the center of RecyclerView, it only respects RecyclerView's padding when clipToPadding is set to true. However, even when clipToPadding is set to true, the center should be in the middle between the padded edges, not in the middle between the actual edges.

The reasoning is that the padding defines the content rectangle, and whether or not children's pixels are clipped to that rectangle or not doesn't change the semantics of that rectangle.

Consider for example a horizontal ViewPager2 that has unequal padding on the left and the right. When it is in idle state, a page exactly fills the rectangle described by the inner edge of the padding, so the page looks to be off-center by design. When snapping kicks in (e.g., because of a swipe), the page to which it is snapping should end up in that same "off-center" position, exactly matching the rectangle described by the inner edge of the padding, regardless of the value of clipToPadding.

Comments

je...@google.com <je...@google.com> #2Aug 15, 2019 02:51PM

Oh I forgot to mention that I thought it might be connected to this issue: https://issuetracker.google.com/issues/243778589. But after trying 2.5.2 I can say that the issue persists.

sh...@google.com <sh...@google.com> #3Aug 15, 2019 11:39PM

Please provide a minimal sample project along with the minimal steps to recreate the issue in the project.

je...@google.com <je...@google.com> #4Aug 16, 2019 04:57PM

Reassigned to je...@google.com.

Sorry for the delay. I got a working example here: https://github.com/Del-S/issues. The project is set up to crash when navigating to ThirdFragment (nested nav graph) with poping the graph.

I poked into it a little bit and it seems to be connected to SafeArgs gradle plugin. When safe args is used to navigate to ThirdFragment (with nested graph poped) it will try to recreate te parent but there were no arguments passed so it crashes.

Steps to reproduce:

Navigate from Second Fragment to Child nav graph (with non-nullable parameters).
Navigate to Third fragment using SafeArgs and the app crashes.

Crash log:

Process: cz.dels.issues, PID: 1743
java.lang.NullPointerException: null cannot be cast to non-null type kotlin.Long
at androidx.navigation.NavType$Companion$LongType$1.get(NavType.kt:352)
at androidx.navigation.NavType$Companion$LongType$1.get(NavType.kt:342)
at androidx.navigation.NavArgument.verify(NavArgument.kt:76)
at androidx.navigation.NavDestination.addInDefaultArgs(NavDestination.kt:502)
at androidx.navigation.NavController.addEntryToBackStack(NavController.kt:1865)
at androidx.navigation.NavController.addEntryToBackStack$default(NavController.kt:1813)
at androidx.navigation.NavController$navigate$4.invoke(NavController.kt:1721)
at androidx.navigation.NavController$navigate$4.invoke(NavController.kt:1719)
at androidx.navigation.NavController$NavControllerNavigatorState.push(NavController.kt:287)
at androidx.navigation.fragment.FragmentNavigator.navigate(FragmentNavigator.kt:246)
at androidx.navigation.fragment.FragmentNavigator.navigate(FragmentNavigator.kt:162)
at androidx.navigation.NavController.navigateInternal(NavController.kt:260)
at androidx.navigation.NavController.navigate(NavController.kt:1719)
at androidx.navigation.NavController.navigate(NavController.kt:1545)
at androidx.navigation.NavController.navigate(NavController.kt:1472)
at androidx.navigation.NavController.navigate(NavController.kt:1930)
at cz.dels.issues.SecondFragment.onViewCreated$lambda-0(SecondFragment.kt:38)
at cz.dels.issues.SecondFragment.$r8$lambda$XDYnOS_cYrafiNQ5rcCu1WCn0IE(Unknown Source:0)
at cz.dels.issues.SecondFragment$$ExternalSyntheticLambda0.onClick(Unknown Source:2)

Note: If in step 2 SaveArgs is not used then navigation works correctly. More information is here: https://github.com/Del-S/issues/blob/master/app/src/main/java/cz/dels/issues/SecondFragment.kt#L37.

je...@google.com <je...@google.com> Aug 20, 2019 02:53PM

Accepted by je...@google.com.

ap...@google.com <ap...@google.com> #5Aug 21, 2019 11:57PM

Ups a typo: Navigate from ~~Second~~ First Fragment to Child nav graph (with non-nullable parameters).

Note: sorry for the spam but I am not able to edit my own comment.

je...@google.com <je...@google.com> #6Aug 22, 2019 10:00AM

Status: Assigned (reopened)

This has been fixed and will be available in navigation 2.6.0-alpha08

ap...@google.com <ap...@google.com> #7Aug 22, 2019 04:54PM

Project: platform/frameworks/support
Branch: androidx-main

commit 6b358154b794a0456b089ac8e548bfb830dd6c22
Author: Clara Fok <clarafok@google.com>
Date: Tue Mar 14 17:56:12 2023

Fix missing non-nullable arg when rebuilding hierarchy

When navigating with NavDirections, args is populated with an empty bundle. This causes issue when we rebuild parent hierarchy while adding a new entry to NavBackStack. If the Entry being rebuilt contains a non-nullalbe arg, i.e. Long, this empty bundle will cause an exception.

Test: ./gradlew navigation:navigation-runtime:cC
Bug: 249988437
Change-Id: I5c8ce739ad9a3428c8a8de13eae391bfff0db5df

M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerTest.kt
M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt

https://android-review.googlesource.com/2488855

je...@google.com <je...@google.com> Aug 27, 2019 09:54AM

Marked as fixed.