IssueTracker

Diego, currently computeCurrentWindowMetrics requires a @UiContext and will crash if the provided context cannot be unwrapped to an Activity or InputMethodService.

For views that are created without a UiContext, such as in a robolectric test, or when inflated using the applicationContext (for example to be added to window manager directly with WM#addView), should computeCurrentWindowMetrics implement some fallback behavior (such as size of the ViewRootImpl, or default display size)? If not then consumers of this API will need to define their own fallbacks which will lead to inconsistency

Do you mind providing code samples and then I can look into it further.

Sure, something like this: (make sure to run it on a device below R)

class MainActivity : ComponentActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        setContentView(Button(this).apply {
            setText("Hello world")
            setOnClickListener {
                addOverlayView()
            }
        })
    }

    fun addOverlayView() {
        val view = MyCustomView(applicationContext).apply {
            updateWindowSizeClass()
        }
        val params = WindowManager.LayoutParams(
            WindowManager.LayoutParams.WRAP_CONTENT,
            WindowManager.LayoutParams.WRAP_CONTENT,
            WindowManager.LayoutParams.TYPE_APPLICATION_SUB_PANEL,
            WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE,
            PixelFormat.TRANSLUCENT
        ).apply { token =
            window.decorView.getRootView().windowToken
        }
        val windowManager = getSystemService(WINDOW_SERVICE) as WindowManager
        windowManager.addView(view, params)
    }
}

class MyCustomView(context: Context) : TextView(context) {
    fun updateWindowSizeClass() {
        val metrics = WindowMetricsCalculator.getOrCreate().computeCurrentWindowMetrics(context)
        setText("Width = ${metrics.bounds.width()} height = ${metrics.bounds.height()}")
    }
}

This works above R, but below R it crashes with:

java.lang.IllegalArgumentException: Context android.app.Application@20b1a86 is not a UiContext

An application Context is not a UI Context so it isn't guaranteed to work. We could add stricter enforcement but that would make it so that the method throws when using an application context.

An application Context is not a UI Context because it is not associated with a Display. Therefore creating UI with it is not recommended.

Sorry if I wasn't clear, but we don't control the callers here, it is common (and supported) for developers to create views with an application context. The question is what should the window size we provide in Compose be in this case. And whatever logic we decide to do there (e.g root view size, default display size) probably makes more sense to implement inside window metrics calculator instead of crashing, so we can be consistent in the fallback behavior across applications and libraries.

Otherwise we would have to basically rely on the implementation detail that the WM library tries to unwrap the context, so we would do it first to see if we can do it safely, and only if so call the WM API

Curious if you have any thoughts as to this, as there is still a 'window' somewhere in the app that is meaningful for UI decisions based on window size

There are no correct window metrics that you can get from the Application Context. There is some parts leftover for compatibility reasons but for new code paths one should not use the Application Context to create Views. For example, there is no default display size because Context#getDisplay will crash. You will not get a correct result going forward when using Context.getSystemService(WindowManager::class.java).getDefaultDisplay().

as there is still a 'window' somewhere in the app that is meaningful for UI decisions based on window size

Where is the window?

Also, in your example above why not use the Activity as the Context?

There are no correct window metrics that you can get from the Application Context

Above R it seems to work fine, since it uses WindowManager#getCurrentWindowMetrics, but I guess there is nothing else we can use below R

You will not get a correct result going forward when using Context.getSystemService(WindowManager::class.java).getDefaultDisplay().

Right, but it's a better approximation than crashing, we need to provide our consumers with some value here. We would only need to do this below R, if the context is not a UI context anyway.

Where is the window?

The same entity that WindowManager#getCurrentWindowMetrics returns above R for an applicationContext, not sure how this is calculated but it seems to map to the current activity inside the application.

Also, in your example above why not use the Activity as the Context?

My example is a bit contrived, in reality this is a common pattern internally. We tried to land a change that calculates default window metrics from the context, and this crashed in multiple tests because the applicationContext was being used, and it's not feasible to update all these existing applicationContext usages that work fine outside of calculating window size.

In any case if there is no desire to avoid this crash at the window library layer, we'll have to land some workaround instead for Compose

Above R it seems to work fine, since it uses WindowManager#getCurrentWindowMetrics, but I guess there is nothing else we can use below R

Not crashing isn't the same as working fine. WindowManager#getCurrentWindowMetrics would be incorrect in a multi display environment.

Right, but it's a better approximation than crashing, we need to provide our consumers with some value here. We would only need to do this below R, if the context is not a UI context anyway.

I don't agree since he default display is the phone's display which can be different from a 42" 4k monitor. And like I said, a non UI context should not be used.

The same entity that WindowManager#getCurrentWindowMetrics returns above R for an applicationContext, not sure how this is calculated but it seems to map to the current activity inside the application.

The problem is when you are in a multi display environment and that is when it would break.

My example is a bit contrived, in reality this is a common pattern internally. We tried to land a change that calculates default window metrics from the context, and this crashed in multiple tests because the applicationContext was being used, and it's not feasible to update all these existing applicationContext usages that work fine outside of calculating window size.

Why are you all using the Application Context instead of a proper UI Context?

Not crashing isn't the same as working fine. WindowManager#getCurrentWindowMetrics would be incorrect in a multi display environment.

Isn't this what is normally used by WindowMetricsCalculator above R?

https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:window/window/src/main/java/androidx/window/layout/util/WindowMetricsCompatHelper.kt;l=88

On a UI Context it is correct, on a non-UI Context it is incorrect.

Discussed more offline, filed b/360934048 as a feature request for WM library to support applicationContext

Note we'll also need to address the performance regression with the original change: b/360151053

Filed a bug for the performance issue, which was rootcaused to insets calculation in WindowMetricsCalculator - since we don't need insets in what we are trying to provide, for now we can just fork the bounds logic, and unify with window library when these blocking issues are resolved

Project: platform/frameworks/support
Branch: androidx-main
Author: Louis Pullen-Freilich <lpf@google.com>
Link:      https://android-review.googlesource.com/3278834

Re-land adding WindowInfo#containerSize

Re-land adding WindowInfo#containerSize

Temporarily using a forked subset of WindowMetricsCalculator to avoid performance issues with inset calculations (that we don't need).

Bug: b/369334429
Fixes: b/360343819
Test: ComposeViewTest
Test: WindowInfoCompositionLocalTest
Relnote: "Adds WindowInfo#containerSize to provide the current window's content container size. This can be retrieved using LocalWindowInfo."
Change-Id: I277673b5576f15f60bc82eeb9d9424c5144a3c25

Project: platform/frameworks/support
Branch: androidx-main
Author: Louis Pullen-Freilich <lpf@google.com>
Link:      https://android-review.googlesource.com/3290009

Re-re-land adding WindowInfo#containerSize

Re-re-land adding WindowInfo#containerSize

Temporarily using a forked subset of WindowMetricsCalculator to avoid some extra performance issues with inset calculations (that we don't need).

Bug: b/369334429
Fixes: b/360343819
Test: ComposeViewTest
Test: WindowInfoCompositionLocalTest
Relnote: "Adds WindowInfo#containerSize to provide the current window's content container size. This can be retrieved using LocalWindowInfo."
Change-Id: Idc38c705655c9181022161927275318fba86bed8

The following release(s) address this bug.It is possible this bug has only been partially addressed:

• androidx.compose.ui:ui:1.8.0-alpha04
• androidx.compose.ui:ui-android:1.8.0-alpha04
• androidx.compose.ui:ui-jvmstubs:1.8.0-alpha04
• androidx.compose.ui:ui-linuxx64stubs:1.8.0-alpha04

Blocked by a private issue 😪

Agree with #23. It would be ideal to have the tools and implement them on a case-by-case for some of our more complex components. I don't believe anyone wants to make inconsistent behaviour for end-users but getting to that consistent behaviour should be available to us if the default implementation can't reach it in this case. It's problematic for us when our expected traversal order in design (which may be set by accessibility analysts) doesn't meet our actual implementation and sometimes the design/implementation refactor costs are high.

While not on compose, there's been issues in the past with traversal where even with refactors we couldn't get an expected result leading to difficult decisions, which is not ideal for end-users.

Let us take responsibility for meeting the compliance, consistency, and accessibility of our own implementation.

Thanks for the feedback. What I would find helpful to make progress here is to hear the concrete cases you're running into where the default order deviates from the one your a11y analysts recommend. Feel free to file those as separate bugs under the following component: https://issuetracker.google.com/issues/new?component=1070466

Then, case by case, we could: 1) suggest a technical workaround to make the order conform without redesigning your app, 2) fix our implementation so that the default order conforms to what you want, or 3) acknowledge that it is technically impossible right now and block those bugs on this feature request. If we wind up accumulating a critical mass of case 3), that would justify putting this on our roadmap, as well as making sure that it supports all the motivating use cases.

Here is an example:

I have a list item (see screenshot) where there is Text, Detail Text, and Meta text. I want these to be announced in that same order.

However, talkback announces the order by default like "Text, Meta text, Detail text".

I believe this is because my layout is like this:

Column {
    Row { 
        Text()
        MetaText()
    }
    DetailText()
}

So I think the only way I could workaround this is by creating a custom layout that places the composables in the same order that they should be read out loud in talkback. But this isn't very convenient.

Another use case: https://github.com/google/accompanist/issues/1254

In short, there is a topBar in a Scaffold that is being placed in the same location as the content slot (and the content is given the space that the top bar occupies as padding values via subcomposition)

Since topBar and content are placed at the same location, the logic in SemanticsSort eventually choose the content first, since it is a taller element: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/semantics/SemanticsSort.kt;l=130;drc=627183600bfd9920c47bd26bdfe5ec906b54d2a7

That is unexpected, however, since the topBar will visually be the top-most element. There doesn't appear to be any way currently to affect this ordering, other than by changing where the topBar and content are placed.

The same issue is also currently affecting the M3 Scaffold for the same reason.

Indeed, so far, more than half the focus-traversal problem cases I've heard involve a top bar. We'll see what we can do to improve that particular use case.

Here is another example where we think it would be nice to have an API to specify the custom accessibility focus order. Have a look at this repo. We have a LazyRow of tabs and a HorizontalPager. When used with TalkBack, the user has to scroll through all the pages back to the first element of the first page to finally jump out of the Pager and to return to the tabs. The tabs are there to quicken the navigation (the user selects a tab, then the Pager goes straight to its content). So we think it would be nice to allow the user to exit from the current page in the middle of the Pager straight to the active tab - not to the previous page. Maybe, it would also be helpful to automatically focus the page right after the user selects a tab, so it is not required to scroll through all the rest tabs in LazyRow to reach the now-active page.

Hello, anybody can show a use case of:

modifier = modifier
        .semantics {
            requestFocus(label = "label", action = {
                anyAction()
                true
            } ...

For handle talkback focus

Or:

modifier = modifier
            .semantics {
                focused = false
               ...
to lost a focus and recovery again.

Thanks!

Any news on when this fix will actually hit a release? The changes look beautiful!

The new traversalIndex/traversalGroup SemanticsProperties will first be released in 1.5.0-beta01, scheduled for mid-next-week. We think it should be a flexible tool to improve a11y focus order for many of the known use cases, I hope it solves your problem!

If it doesn't behave as you expect, please post a video along with sample code, and we'll investigate. Also, please note that there are still known issues with TalkBack initial focus after navigation which traversalIndex is not yet able to solve (see b/272065229), so we plan to continue to add more features in this area in Compose 1.6.

Greatly appreciated. My use case will definitely put it to the test!

Thank you so much for the travesalIndex/traversalGroup release! It has mostly worked as expected. The one issue I am seeing is when the parent has semantics(mergeDescendants = true) {}. I made an example:

class MainActivity : ComponentActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent {
            TraveralTestTheme {
                TraversalExample(
                    modifier = Modifier
                        .semantics(mergeDescendants = true) {  }
                        .background(color = Color.Gray)
                        .fillMaxSize()
                )
            }
        }
    }
}

@Composable
fun TraversalExample(
    modifier: Modifier = Modifier,
) {
    Row(
        horizontalArrangement = Arrangement.Center,
        verticalAlignment = Alignment.CenterVertically,
        modifier = modifier.semantics { isTraversalGroup = true },
    ){
        Column {
            Text(
                text = "I should be announced first",
                modifier = Modifier
                    .padding(16.dp)
                    .semantics { traversalIndex = 1f },
            )
            Text(
                text = "I should be announced third",
                modifier = Modifier
                    .padding(16.dp)
                    .semantics { traversalIndex = 3f },
            )
        }
        Text(
            text = "I should be announced second",
            modifier = Modifier
                .padding(16.dp)
                .semantics { traversalIndex = 2f },
        )
    }
}

The announcement order is: 1,3,2.

The announced string is: "I should be announced first, I should be announced third, I should be announced second"

Got it. traversalIndex's current implementation only control the order that TalkBack moves its focus box around: we hadn't yet considered the use case of controlling the announcement order within a single merged element. Feel free to file a separate bug on that.

For now, clearAndSetSemantics { contentDescription = "I should be announced first, I should be announced second, I should be announced third" } could be an alternate way of solving this, although I recognize clearAndSetSemantics has downsides and it's usually better to avoid it.

OK. I will do that! Thank you very much. I have noticed other interesting behavior but haven't been as successful in reproducing it. We think some of the behavior is related to this:

Also remove the idea of 'structurally significant' to only talk about traversalGroups by adding isTraversalGroup = true to Foundation classes (Modifier.scroll semantics and LazyLayoutSemantics). This way, all scrollables are considered traversalGroups in preparation of aosp/2491418 and go/traversal-index-changes.

as we have solved some of our issues by using Modifier.semantics { isTraversalGroup = false } with scrollable components. The basic description of the issue "the scrollable item seems to be moved deeper into the traversal tree than its peers as it is skipped until the next layer is announced. However, it is properly announced if isTraversalGroup is set to false".

OK, that sounds like another thing worth filing a separate bug for. We can use your example either to improve our default behavior, or to provide guidance on when to specify isTraversalGroup = false. Please include two videos of what you are seeing (with isTraversalGroup = false on the scrollable, and without).

Of course. Thank you!

I made the issue for that first bug! https://issuetracker.google.com/issues/285043826

Thanks for this long waiting feature. During integration we faced runtime crash when we switch compose screens: java.lang.IllegalStateException: LayoutNode should be attached to an owner

Similar issue: https://issuetracker.google.com/issues/285195974

Marking this bug fixed because the new traversalIndex & traversalGroup SemanticsProperties provide the desired manual control of focus order and they are scheduled for release soon in Compose 1.5.

If anyone still can't find a way to solve their use case after experimenting with them, please file a new bug and post a video along with sample code.