Skip to content

Commit

Permalink
Skip A11Y sanity check for keyboard navigation
Browse files Browse the repository at this point in the history
Summary:
As external keyboard cannot be detected by the current API:

https://www.internalfb.com/code/fbsource/[details]/fbandroid/libraries/components/litho-core/src/main/java/com/facebook/litho/AccessibilityUtils.kt?lines=32-35

It makes Litho will not create `AccessibilityDelegate` for each host whenever A11Y is not enabled, which ends up that keyboard navigation doesn't work at all.

Quote from Brett:
> Otherwise, I think your best bet is to observe connections for keyboards and  change the behavior accordingly. As far as I know this requires a change to the activity in the AndroidManifest, so wouldn’t be something that you could fix in Litho alone. Though most of the activities in the FB4a manifest appear to have the needed property already.

I also ran QE to create host for each component which did see a lot of perf regressions as we expected(https://fburl.com/deltoid3/gfny45o5). It's probably hard for us to ship this.

So for now, I would recommend just skip this sanity check for host, which only works for components that have A11Y enabled as well as focus event handlers. (Otherwise, there's no point to enable keyboard navigation for this component from my understanding.)

Reviewed By: adityasharat

Differential Revision: D69053388

fbshipit-source-id: cfe24bab0948e7f2ef8c1c7544122c956a53c87b
  • Loading branch information
Andrew Wang authored and facebook-github-bot committed Feb 4, 2025
1 parent f2eadd9 commit f42ec59
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 7 deletions.
4 changes: 2 additions & 2 deletions litho-core/src/main/java/com/facebook/litho/ComponentHost.kt
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ open class ComponentHost(
setWillNotDraw(ComponentsConfiguration.defaultInstance.enableHostWillNotDraw)
isChildrenDrawingOrderEnabled = true
refreshAccessibilityDelegatesIfNeeded(
ComponentsConfiguration.enableA11Y || isAccessibilityEnabled(context))
ComponentsConfiguration.skipA11YValidationForKeyboard || isAccessibilityEnabled(context))
}

override fun mount(index: Int, mountItem: MountItem) {
Expand Down Expand Up @@ -438,7 +438,7 @@ open class ComponentHost(
super.setTag(key, tag)
if (key == COMPONENT_NODE_INFO_ID && tag != null) {
refreshAccessibilityDelegatesIfNeeded(
ComponentsConfiguration.enableA11Y || isAccessibilityEnabled(context))
ComponentsConfiguration.skipA11YValidationForKeyboard || isAccessibilityEnabled(context))
if (componentAccessibilityDelegate != null) {
(tag as? NodeInfo)?.let { componentAccessibilityDelegate?.setNodeInfo(it) }
}
Expand Down
3 changes: 1 addition & 2 deletions litho-core/src/main/java/com/facebook/litho/LithoNode.kt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import com.facebook.litho.CommonProps.DefaultLayoutProps
import com.facebook.litho.ComponentHostUtils.maybeSetDrawableState
import com.facebook.litho.Transition.TransitionKeyType
import com.facebook.litho.annotations.ImportantForAccessibility
import com.facebook.litho.config.ComponentsConfiguration
import com.facebook.litho.config.LithoDebugConfigurations
import com.facebook.litho.drawable.ComparableColorDrawable
import com.facebook.litho.layout.LayoutDirection
Expand Down Expand Up @@ -1228,7 +1227,7 @@ open class LithoNode : Node<LithoLayoutContext>, Cloneable {
ComponentContext.getComponentsConfig(c).shouldAddRootHostViewOrDisableBgFgOutputs &&
(node.background != null || node.foreground != null)
val hasAccessibilityContent =
((context?.isAccessibilityEnabled == true) || ComponentsConfiguration.enableA11Y) &&
(context?.isAccessibilityEnabled == true) &&
(importantForAccessibility != ViewCompat.IMPORTANT_FOR_ACCESSIBILITY_NO) &&
(implementsAccessibility ||
!(nodeInfo?.contentDescription.isNullOrEmpty()) ||
Expand Down
3 changes: 2 additions & 1 deletion litho-core/src/main/java/com/facebook/litho/LithoView.java
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,8 @@ protected void onAttached() {
}

refreshAccessibilityDelegatesIfNeeded(
ComponentsConfiguration.enableA11Y || isAccessibilityEnabled(getContext()));
ComponentsConfiguration.skipA11YValidationForKeyboard
|| isAccessibilityEnabled(getContext()));

AccessibilityManagerCompat.addAccessibilityStateChangeListener(
mAccessibilityManager, mAccessibilityStateChangeListener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,10 @@ internal constructor(
@JvmField var useUndefinedYogaValue: Boolean = false

/**
* This flag is used to enable A11Y support for Litho by default to measure the impact of perf.
* This flag is used to enable A11Y support for keyboard navigation, which was disabled due to
* Litho's A11Y sanity check.
*/
@JvmField var enableA11Y: Boolean = false
@JvmField var skipA11YValidationForKeyboard: Boolean = false

/** This flag is used clear MovementMethod on TextInput to avoid a potential crash. */
@JvmField var clearMovementMethod: Boolean = true
Expand Down

0 comments on commit f42ec59

Please sign in to comment.