It seems there's a bug in Compose when navigating ...

# compose

[Ondřej Kycelt]

It seems there's a bug in Compose when navigating focus within grid layouts. In certain situations, a row is skipped when moving focus up or down. Specifically, if an item is significantly wider than the currently focused item and both share the same start or end x coordinate, the focus search skips it and jumps to another row. The minimal repro below uses LazyVerticalGrid, but I'm encountering the same issue with a more complex layout based on MinaBox. Has anyone else experienced this issue? Is there any workaround other than adjusting item sizes?

@Composable
private fun TestFocusSearch(
  modifier: Modifier = Modifier
) {
  LazyVerticalGrid(
    columns = GridCells.Fixed(4),
    contentPadding = PaddingValues(horizontal = 64.dp, vertical = 48.dp),
    horizontalArrangement = Arrangement.spacedBy(16.dp),
    modifier = modifier,
    verticalArrangement = Arrangement.spacedBy(16.dp)
  ) {
    items(12) {
      FocusableBox()
    }

    item(
      span = { GridItemSpan(maxLineSpan) }
    ) {
      FocusableBox()
    }

    items(12) {
      FocusableBox()
    }
  }
}

@Composable
private fun FocusableBox(
  modifier: Modifier = Modifier
) {
  var isFocused by remember { mutableStateOf(false) }

  Box(
    modifier = modifier
      .height(40.dp)
      .drawBehind {
        drawRect(if (isFocused) Color.Red else Color.Gray)
      }
      .onFocusChanged { isFocused = it.isFocused }
      .focusable()
  )
}

focus_search.webm

[Zach Klippenstein (he/him) [MOD]]

Please keep long code snippets to the thread. I would file a bug for this

[Ondřej Kycelt]

Sorry, I'll keep that in mind. I've filed a bug here https://issuetracker.google.com/issues/367440163

[Ondřej Kycelt]

I’ve managed to track down the issue in TwoDimensionalFocusSearch.kt and tried to fix it via https://android-review.googlesource.com/c/platform/frameworks/support/+/3271691. It’s my first attempt at a contribution, so not sure how the process works exactly. May I ask how long it usually takes for somebody to look at it?

[Zach Klippenstein (he/him) [MOD]]

You typically need to work with someone on the inside to land it. Cc @Ralston Da Silva

[Ondřej Kycelt]

So reaching out to someone here on Slack can normally be the way to go?

[Zach Klippenstein (he/him) [MOD]]

Someone from the team usually sees it eventually

[Ralston Da Silva]

Took a look at this CL. Thanks for debugging and sending the fix Ondrej!

Compose uses the same value that we used for Android Views and changing it might make the behavior inconsistent. I think we could land this as in the provided example seems like a valid use-case. If it causes some regression we could revert it. Checking with a couple of people before approving the CL.

[Ralston Da Silva]

I noticed the sample code works in portrait mode without any changes and the fix is only needed in landscape mode. This makes me feel like adjusting the weight for the major axis might not fix this for all cases.

We could fix this by specifying a custom focus order:

@Composable
fun TestFocusSearch(
    modifier: Modifier = Modifier
) {
    val columnCount = 4
    fun isLastLine(itemIndex: Int, itemCount: Int): Boolean = itemIndex >= itemCount - columnCount
    fun isFirstLine(itemIndex: Int) = itemIndex < columnCount
    val middleItem = remember { FocusRequester() }
    
    LazyVerticalGrid(
        columns = GridCells.Fixed(columnCount),
        contentPadding = PaddingValues(horizontal = 64.dp, vertical = 48.dp),
        horizontalArrangement = Arrangement.spacedBy(16.dp),
        modifier = modifier,
        verticalArrangement = Arrangement.spacedBy(16.dp)
    ) {

        items(12) {
            if (isLastLine(it, 12)) {
                FocusableBox(Modifier.focusProperties{ down = middleItem })
            } else {
                FocusableBox()
            }
        }

        item(span = { GridItemSpan(maxLineSpan) }) {
            FocusableBox(Modifier.focusRequester(middleItem))
        }

        items(12) {
            if(isFirstLine(it)) {
                FocusableBox(Modifier.focusProperties{ up = middleItem })
            } else {
                FocusableBox()
            }
        }
    }
}

The benefit to this approach is that you get more control of the focus traversal order. For instance, the example video indicates another issue. Once the large item is focused pressing up/down always takes you to the 2nd column regardless of where focus came from (before the large item was focused). This can be fixed by using focus groups and specifying a custom exit property (An alternate approach would be to use onFocusChanged and the down/up focus property)

@Composable
fun TestFocusSearch(
    modifier: Modifier = Modifier
) {
    var nextUpItem: FocusRequester? = null
    var nextDownItem: FocusRequester? = null

    val columnCount = 4
    fun isLastLine(itemIndex: Int, itemCount: Int): Boolean = itemIndex >= itemCount - columnCount
    fun isFirstLine(itemIndex: Int) = itemIndex < columnCount

    val before = remember{ Array(columnCount) { FocusRequester() } }
    val after = remember{ Array(columnCount) { FocusRequester() } }
    val middleItem = remember { FocusRequester() }

    LazyVerticalGrid(
        columns = GridCells.Fixed(columnCount),
        contentPadding = PaddingValues(horizontal = 64.dp, vertical = 48.dp),
        horizontalArrangement = Arrangement.spacedBy(16.dp),
        modifier = modifier,
        verticalArrangement = Arrangement.spacedBy(16.dp)
    ) {
        items(12) { index ->
            if (isLastLine(index, 12)) {
                FocusableBox(
                    Modifier
                        .focusProperties{
                            exit = {
                                if (it == Down) {
                                    nextUpItem = before[index%columnCount]
                                    nextDownItem = after[index%columnCount]
                                    middleItem
                                } else {
                                    Default
                                }
                            }
                        }
                        .focusGroup()
                        .focusRequester(before[index%columnCount])
                )
            } else {
                FocusableBox()
            }
        }

        item(span = { GridItemSpan(maxLineSpan) }) {
            FocusableBox(
                Modifier
                    .focusProperties {
                        exit = {
                            when(it) {
                                Up -> nextUpItem
                                Down -> nextDownItem
                                else -> null
                            } ?: Default
                        }
                    }
                    .focusGroup()
                    .focusRequester(middleItem)
            )
        }

        items(12) { index ->
            if(isFirstLine(index)) {
                FocusableBox(
                    Modifier
                        .focusProperties{
                            exit = {
                                if (it == Up) {
                                    nextUpItem = before[index%columnCount]
                                    nextDownItem = after[index%columnCount]
                                    middleItem
                                } else {
                                    Default
                                }
                            }
                        }
                        .focusGroup()
                        .focusRequester(after[index%columnCount])
                )
            } else {
                FocusableBox()
            }
        }
    }
}

Here is what this looks like:

LargeItemInGrid.webm

[Ondřej Kycelt]

If it's a simple grid with a few items of a known structure, I would also just specify a custom focus order as you suggest and be done with it. Unfortunately, the problem I'm trying to solve is a bit more complex. To give more context, we've encountered this issue when building a TV guide layout for a TV app (see the attached video). Program cards in different channel rows have different positions and different widths, because not every program starts at the same time and has the same duration. The data set I'm testing with has 72 channels and 1816 programs, so if I'd like to specify a custom focus order, I would have to create 1816 focus requesters and somehow pre-calculate up and down for each program. I think handling these edge cases in 2D focus search would be much simpler and likely more performant, especially on TVs that usually have lower performance than phones.

Btw. the layout is based on MinaBox (https://github.com/oleksandrbalan/minabox) which is a 2D draggable/scrollable LazyLayout.

tvguide_focus.webm

[Ondřej Kycelt]

I noticed the sample code works in portrait mode without any changes and the fix is only needed in landscape mode. This makes me feel like adjusting the weight for the major axis might not fix this for all cases.

The sample code works in portrait mode on a phone, because the full-span item isn't wide enough for the focus search algorithm to skip it. If the item height is smaller, the full-span item is skipped even on a phone in portrait mode (see the attached video).

focus_phone_portrait.webm

[Ondřej Kycelt]

@Ralston Da Silva, just checking in - have you had a chance to think about this more?

[Ralston Da Silva]

I spoke to people on the team, and we feel like we shouldn't change the weights since the current values matches what we have for AndroidViews. The larger issue here is that it would fix the issue where the large cell is not visited from column 1, but it wont fix the issue when we leave the large cell. Focus would move to column 2 instead of column 1. This feels like we are optimizing for a specific use-case, and so we shouldn't change the default behavior.

[Ralston Da Silva]

We could make this a feature request, and add a focusProperty that allows us to fine tune the behavior.

[Ondřej Kycelt]

In that case, I would argue that the value is also incorrect in AndroidViews. Also, I feel like focus moving to column 2 instead of column 1 is not really an issue since focus search is looking for the closest item to focus in a given direction and doesn't take into consideration the path the focus took before.

[Ondřej Kycelt]

As for the feature request, do you mean the ability to specify the weight via focusProperties ? If so, is it something that can be done "quickly"?

[Ondřej Kycelt]

I'm asking because we'd really like to update to Compose 1.7 and this is the last blocking issue.

[Ralston Da Silva]

Unfortunately we can't add APIs to 1.7 at this point. We would have to do this for 1.8

[Ondřej Kycelt]

1.8 is probably too late for us. I've been trying the approach with FocusRequester s, specifying up and down focus property for each program, and so far it seems to perform okay even for 800+ channels, so we're probably gonna go that way. Thanks for your time 🙂