From ffc4bc77b876db7b34fe97eb69a03c1d7df7db60 Mon Sep 17 00:00:00 2001
From: Yining Liu <liuyining@google.com>
Date: Thu, 8 Sep 2022 20:03:39 +0000
Subject: [PATCH] Fix the order of talkback focus for ConversationLayout when
 expanded

Talkback can now correctly focus the collapse button before going to the action list when expanded. To know why mExpandButton is dynamically placed, check commit: 54fe9ad2dc421256773285f9212132aec3abcb4b

Bug: 237505828
Test: Visual, see the STEPS TO REPRODUCE section of bug 237505828, the focus order of talkback should be collapse button then actions.
Change-Id: Id0cf8730b00e0e7636a163fb5d51e37e8e0a3e84
---
 .../internal/widget/ConversationLayout.java   |  5 +-
 ...ication_template_material_conversation.xml | 79 +++++++++++--------
 core/res/res/values/symbols.xml               |  1 +
 3 files changed, 53 insertions(+), 32 deletions(-)

diff --git a/core/java/com/android/internal/widget/ConversationLayout.java b/core/java/com/android/internal/widget/ConversationLayout.java
index 4706affa3541a..5f8acff80c0a9 100644
--- a/core/java/com/android/internal/widget/ConversationLayout.java
+++ b/core/java/com/android/internal/widget/ConversationLayout.java
@@ -111,6 +111,7 @@ public class ConversationLayout extends FrameLayout
     private Icon mLargeIcon;
     private View mExpandButtonContainer;
     private ViewGroup mExpandButtonAndContentContainer;
+    private ViewGroup mExpandButtonContainerA11yContainer;
     private NotificationExpandButton mExpandButton;
     private MessagingLinearLayout mImageMessageContainer;
     private int mBadgeProtrusion;
@@ -234,6 +235,8 @@ public class ConversationLayout extends FrameLayout
         });
         mConversationText = findViewById(R.id.conversation_text);
         mExpandButtonContainer = findViewById(R.id.expand_button_container);
+        mExpandButtonContainerA11yContainer =
+                findViewById(R.id.expand_button_a11y_container);
         mConversationHeader = findViewById(R.id.conversation_header);
         mContentContainer = findViewById(R.id.notification_action_list_margin_target);
         mExpandButtonAndContentContainer = findViewById(R.id.expand_button_and_content_container);
@@ -1091,7 +1094,7 @@ public class ConversationLayout extends FrameLayout
             newContainer = mExpandButtonAndContentContainer;
         } else {
             buttonGravity = Gravity.CENTER_HORIZONTAL | Gravity.TOP;
-            newContainer = this;
+            newContainer = mExpandButtonContainerA11yContainer;
         }
         mExpandButton.setExpanded(!mIsCollapsed);
 
diff --git a/core/res/res/layout/notification_template_material_conversation.xml b/core/res/res/layout/notification_template_material_conversation.xml
index 42fb4a26dd3b6..ce8a904955727 100644
--- a/core/res/res/layout/notification_template_material_conversation.xml
+++ b/core/res/res/layout/notification_template_material_conversation.xml
@@ -89,45 +89,62 @@
         <include layout="@layout/notification_material_action_list" />
     </com.android.internal.widget.RemeasuringLinearLayout>
 
-    <!--This is dynamically placed between here and at the end of the layout. It starts here since
-        only FrameLayout layout params have gravity-->
+    <!--expand_button_a11y_container ensures talkback focus order is correct when view is expanded.
+    The -1px of marginTop and 1px of paddingTop make sure expand_button_a11y_container is prior to
+    its sibling view in accessibility focus order.
+    {see android.view.ViewGroup.addChildrenForAccessibility()}
+    expand_button_container will be moved under expand_button_and_content_container when collapsed,
+    this dynamic movement ensures message can flow under expand button when expanded-->
     <FrameLayout
-        android:id="@+id/expand_button_container"
-        android:layout_width="wrap_content"
+        android:id="@+id/expand_button_a11y_container"
+        android:layout_width="match_parent"
         android:layout_height="match_parent"
         android:layout_gravity="end|top"
         android:clipChildren="false"
-        android:clipToPadding="false">
-        <!--This layout makes sure that we can nicely center the expand content in the
-            collapsed layout while the parent makes sure that we're never laid out bigger
-            than the messaging content.-->
-        <LinearLayout
-            android:id="@+id/expand_button_touch_container"
+        android:clipToPadding="false"
+        android:layout_marginTop="-1px"
+        android:paddingTop="1px"
+        >
+        <!--expand_button_container is dynamically placed between here and at the end of the
+        layout. It starts here since only FrameLayout layout params have gravity-->
+        <FrameLayout
+            android:id="@+id/expand_button_container"
             android:layout_width="wrap_content"
-            android:layout_height="@dimen/conversation_expand_button_height"
-            android:orientation="horizontal"
+            android:layout_height="match_parent"
             android:layout_gravity="end|top"
-            android:paddingEnd="0dp"
-            android:clipToPadding="false"
             android:clipChildren="false"
-            >
-            <!-- Images -->
-            <com.android.internal.widget.MessagingLinearLayout
-                android:id="@+id/conversation_image_message_container"
-                android:forceHasOverlappingRendering="false"
-                android:layout_width="40dp"
-                android:layout_height="40dp"
-                android:layout_marginStart="@dimen/conversation_image_start_margin"
-                android:spacing="0dp"
-                android:layout_gravity="center"
+            android:clipToPadding="false">
+            <!--expand_button_touch_container makes sure that we can nicely center the expand
+            content in the collapsed layout while the parent makes sure that we're never laid out
+            bigger than the messaging content.-->
+            <LinearLayout
+                android:id="@+id/expand_button_touch_container"
+                android:layout_width="wrap_content"
+                android:layout_height="@dimen/conversation_expand_button_height"
+                android:orientation="horizontal"
+                android:layout_gravity="end|top"
+                android:paddingEnd="0dp"
                 android:clipToPadding="false"
                 android:clipChildren="false"
-                />
-            <include layout="@layout/notification_expand_button"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:layout_gravity="center"
-                />
-        </LinearLayout>
+                >
+                <!-- Images -->
+                <com.android.internal.widget.MessagingLinearLayout
+                    android:id="@+id/conversation_image_message_container"
+                    android:forceHasOverlappingRendering="false"
+                    android:layout_width="40dp"
+                    android:layout_height="40dp"
+                    android:layout_marginStart="@dimen/conversation_image_start_margin"
+                    android:spacing="0dp"
+                    android:layout_gravity="center"
+                    android:clipToPadding="false"
+                    android:clipChildren="false"
+                    />
+                <include layout="@layout/notification_expand_button"
+                    android:layout_width="wrap_content"
+                    android:layout_height="wrap_content"
+                    android:layout_gravity="center"
+                    />
+            </LinearLayout>
+        </FrameLayout>
     </FrameLayout>
 </com.android.internal.widget.ConversationLayout>
diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml
index e773a9c706b0e..a6174e146367f 100644
--- a/core/res/res/values/symbols.xml
+++ b/core/res/res/values/symbols.xml
@@ -4279,6 +4279,7 @@
   <java-symbol type="id" name="conversation_icon_badge_ring" />
   <java-symbol type="id" name="conversation_icon_badge_bg" />
   <java-symbol type="id" name="expand_button_container" />
+  <java-symbol type="id" name="expand_button_a11y_container" />
   <java-symbol type="id" name="expand_button_touch_container" />
   <java-symbol type="id" name="messaging_group_content_container" />
   <java-symbol type="id" name="expand_button_and_content_container" />