Skip to content

Add more jank tracking #1833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add more jank tracking #1833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49210d9
Add more jank tracking
AdnanHabibMirza Mar 1, 2025
64e3256
Revert formatting
AdnanHabibMirza Mar 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ...ests/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/TabContent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollba
import com.google.samples.apps.nowinandroid.core.designsystem.component.scrollbar.scrollbarState
import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic
import com.google.samples.apps.nowinandroid.core.ui.InterestsItem
import com.google.samples.apps.nowinandroid.core.ui.TrackScrollJank

@Composable
fun TopicsTabContent(
Expand All @@ -56,10 +57,14 @@ fun TopicsTabContent(
.fillMaxWidth(),
) {
val scrollableState = rememberLazyListState()
val testTag = "interests:topics"

TrackScrollJank(scrollableState = scrollableState, stateName = testTag)

LazyColumn(
modifier = Modifier
.padding(horizontal = 24.dp)
.testTag("interests:topics"),
.testTag(testTag),
contentPadding = PaddingValues(vertical = 16.dp),
state = scrollableState,
) {
Expand Down
20 changes: 16 additions & 4 deletions ...earch/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/search/SearchScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import androidx.compose.foundation.layout.windowInsetsPadding
import androidx.compose.foundation.layout.windowInsetsTopHeight
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.items
import androidx.compose.foundation.lazy.rememberLazyListState
import androidx.compose.foundation.lazy.staggeredgrid.LazyVerticalStaggeredGrid
import androidx.compose.foundation.lazy.staggeredgrid.StaggeredGridCells
import androidx.compose.foundation.lazy.staggeredgrid.StaggeredGridItemSpan
Expand Down Expand Up @@ -92,6 +93,7 @@ import com.google.samples.apps.nowinandroid.core.ui.InterestsItem
import com.google.samples.apps.nowinandroid.core.ui.NewsFeedUiState.Success
import com.google.samples.apps.nowinandroid.core.ui.R.string
import com.google.samples.apps.nowinandroid.core.ui.TrackScreenViewEvent
import com.google.samples.apps.nowinandroid.core.ui.TrackScrollJank
import com.google.samples.apps.nowinandroid.core.ui.newsFeed
import com.google.samples.apps.nowinandroid.feature.search.R as searchR

Expand Down Expand Up @@ -154,8 +156,7 @@ internal fun SearchScreen(
-> Unit

SearchResultUiState.SearchNotReady -> SearchNotReadyBody()
SearchResultUiState.EmptyQuery,
-> {
SearchResultUiState.EmptyQuery -> {
if (recentSearchesUiState is RecentSearchQueriesUiState.Success) {
RecentSearchesBody(
onClearRecentSearches = onClearRecentSearches,
Expand Down Expand Up @@ -293,6 +294,10 @@ private fun SearchResultBody(
onFollowButtonClick: (String, Boolean) -> Unit,
) {
val state = rememberLazyStaggeredGridState()
val testTag = "search:searchResult"

TrackScrollJank(scrollableState = state, stateName = testTag)

Box(
modifier = Modifier
.fillMaxSize(),
Expand All @@ -304,7 +309,7 @@ private fun SearchResultBody(
verticalItemSpacing = 24.dp,
modifier = Modifier
.fillMaxSize()
.testTag("search:newsResources"),
.testTag(testTag),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should update NavigationTest, also why change it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should update NavigationTest

No test dependents were found on search:newsResources.

also why change it?

The composable name is SearchResultBody, but the test tag was incorrectly set to search:newsResources. To align with the correct naming convention, the test tag has been updated to search:searchResult. This ensures consistency and clarity in the codebase.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought I had seen search:newsResources somewhere in test, you right.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if we should handle the test tag naming improvements in a separate PR.

state = state,
) {
if (topics.isNotEmpty()) {
Expand Down Expand Up @@ -394,6 +399,10 @@ private fun RecentSearchesBody(
onRecentSearchClicked: (String) -> Unit,
) {
Column {
val scrollableState = rememberLazyListState()

TrackScrollJank(scrollableState = scrollableState, stateName = "search:recentSearches")

Row(
horizontalArrangement = Arrangement.SpaceBetween,
verticalAlignment = Alignment.CenterVertically,
Expand Down Expand Up @@ -424,7 +433,10 @@ private fun RecentSearchesBody(
}
}
}
LazyColumn(modifier = Modifier.padding(horizontal = 16.dp)) {
LazyColumn(
modifier = Modifier.padding(horizontal = 16.dp),
state = scrollableState,
) {
items(recentSearchQueries) { recentSearch ->
Text(
text = recentSearch,
Expand Down
Loading