Autofocuses input box on Android Leo chat window on a fresh chat #25512

SergeyZhukovsky · 2024-09-10T19:45:13Z

Resolves brave/brave-browser#40968

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on
All relevant documentation has been updated, for instance:

Test Plan:

Screen_recording_20240910_154410.webm

the test plan is specified in the issue

brave-builds · 2024-09-10T23:19:50Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2024-09-10T23:28:03Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2024-09-10T23:29:01Z

A Storybook has been deployed to preview UI for the latest push

fallaciousreasoning

Mostly looks great with a few comments 😄

fallaciousreasoning · 2024-09-11T07:43:24Z

components/ai_chat/resources/page/components/input_box/index.tsx

@@ -76,10 +78,21 @@ function InputBox(props: InputBoxProps) {
    }
  }

+  const querySubmitted = React.useRef(false)


nit: not strictly required but I'd prefer this to be defined before its used (i.e. before handleSubmit)

components/ai_chat/resources/page/components/input_box/index.tsx

fallaciousreasoning · 2024-09-11T07:46:28Z

components/ai_chat/resources/page/components/input_box/index.tsx

+    if (props.context.isMobile && props.context.hasAcceptedAgreement &&
+      !props.context.hasInitialHistory && !querySubmitted.current) {
+        node.focus()
+        getPageHandlerInstance().pageHandler.handleShowSoftKeyboard()


I think this should go through the context so we can bind the input_box to another mojom interface (which I think we do for the AI Rewriter stuff, which currently isn't active)

fallaciousreasoning · 2024-09-11T07:49:40Z

components/ai_chat/resources/page/state/data-context-provider.tsx

@@ -336,6 +336,8 @@ function DataContextProvider(props: DataContextProviderProps) {
  }

  const isMobile = React.useMemo(() => loadTimeData.getBoolean('isMobile'), [])
+  const hasInitialHistory = React.useMemo(() =>


I know you're just following what this file does but I don't think there's any point to this being in a useMemo as:

It isn't an expensive calculation (so we could just store it in a normal variable)

It will never change

Personally, I'd probably store the variable outside the component as

const hasInitialHistory = loadTimeData.getBoolean('hasInitialHistory') function DataContextProvider(...) { ... }

Really this shouldn't be part of the context at all as its completely static data, but I guess its fine as part of the interface the input_box expects.

SergeyZhukovsky · 2024-09-11T17:24:43Z

@fallaciousreasoning the review suggestions are addressed.

petemill · 2024-09-11T21:13:00Z

browser/ui/webui/ai_chat/ai_chat_ui.cc

+  content::WebContents* chat_web_contents = GetChatWebContents();
+  size_t visible_conversation_history_size = 0;
+  if (chat_web_contents) {
+    ai_chat::AIChatTabHelper* active_chat_tab_helper =
+        ai_chat::AIChatTabHelper::FromWebContents(chat_web_contents);
+    visible_conversation_history_size =
+        active_chat_tab_helper->GetVisibleConversationHistory().size();
+  }
+  untrusted_source->AddBoolean("hasInitialHistory",
+                               visible_conversation_history_size != 0);


I don't think this needs to be provided by the C++. The context can just check if the first call to get conversation history has any data.

@petemill what particular call do you mean?

I think @petemill means here where we call getConversationHistory

If its the first call, and we get nothing back, we don't have initial history.

@fallaciousreasoning the getConversationHistory returns 0 always even if we have history.

Surely there's some way that we load the active conversation history because we display it, right?

petemill · 2024-09-11T21:13:46Z

components/ai_chat/resources/page/components/input_box/index.tsx

      node.focus()
    }
+    if (props.context.isMobile && props.context.hasAcceptedAgreement &&


I'm thinking this should be done in a higher level because input is shared with ai rewriter dialog - it doesn't need to be in a conversation with history and doesn't need to have conversation awareness. We can also remove the props.context.isMobile

I can move the check on the higher level but node.focus would be called here depending of true false the higher check function return. The rewriter could just have have a stub with false. Does the rewriter need to be aware of hasAcceptedAgreement as it was there before?

I moved it to the higher level

petemill

Is focus not enugh to show the virtual keyboard? If this is in a regular Tab instead of a custom popup, would we still require a custom HandleShowSoftKeyboard function? I'm asking because wondering if it's worth the effort due to us soon removing the custom popup in favor of a Tab.

SergeyZhukovsky · 2024-09-12T00:52:45Z

Is focus not enugh to show the virtual keyboard? If this is in a regular Tab instead of a custom popup, would we still require a custom HandleShowSoftKeyboard function? I'm asking because wondering if it's worth the effort due to us soon removing the custom popup in favor of a Tab.

I've been fighting with it and it doesn't pop up when I programmatically call focus. The conclusion from other users who tried that was something like that it's generally not possible to programmatically bring up the soft keyboard without some form of user interaction. This is a security feature and accessibility consideration in many mobile operating systems. It's worth effort as we want the soft keyboard to popup regardless are we in a separate window or inside a tab.

SergeyZhukovsky · 2024-09-13T17:44:59Z

I've discussed it with @petemill in slack and we decided to wait with that PR till his is merged #24921

fallaciousreasoning

lgtm % nits

fallaciousreasoning · 2024-09-16T07:30:11Z

components/ai_chat/resources/page/components/input_box/index.tsx

  | 'shouldDisableUserInput'
  | 'hasAcceptedAgreement'>

 interface InputBoxProps {
    context: Props
    onFocusInputMobile?: () => unknown
+    maybeShowSoftKeyboard?: (querySubmitted: boolean) => unknown


hey sorry, I know you're just cargo culting here but I think these methods should return void rather than unknown.

Even if we do return something from these functions its fine, it just means the InputBox doesn't do anything with the return type.

For example, the below is fine:

let foo: ((n: number) => void) = n => n * n

see playground:
https://www.typescriptlang.org/play/?ssl=5&ssc=28&pln=5&pc=29#code/C4TwDgpgBAUghiAzgMQK4DsDGUC8UAU6AXFOqgLYBGEATgJS4B8UAbgPYCWAJgFB8A2EYFABmbNiXhI0WXAXQMczdD0HDKcGpIQoM2PIUXKoAKlJA

fallaciousreasoning · 2024-09-16T07:31:23Z

components/ai_chat/resources/page/state/context.ts

@@ -63,6 +63,7 @@ export interface AIChatContext extends CharCountContext {
  resetSelectedActionType: () => void
  handleActionTypeClick: (actionType: mojom.ActionType) => void
  setIsToolsMenuOpen: (isOpen: boolean) => void
+  handleShowSoftKeyboard?: () => unknown


same here:

Suggested change

handleShowSoftKeyboard?: () => unknown

handleShowSoftKeyboard?: () => void

fallaciousreasoning · 2024-09-16T07:34:45Z

components/ai_chat/resources/page/components/input_box/index.tsx

 }

 function InputBox(props: InputBoxProps) {
  const onInputChange = (e: React.ChangeEvent<HTMLTextAreaElement>) => {
    props.context.setInputText(e.target.value)
  }

+  const querySubmitted = React.useRef(false)


Optional: I'm pretty sure we could use useState here instead of useRef (though not 100%).

fallaciousreasoning · 2024-09-16T07:35:47Z

components/ai_chat/resources/page/components/input_box/index.tsx

@@ -78,21 +80,16 @@ function InputBox(props: InputBoxProps) {
    }
  }

-  const querySubmitted = React.useRef(false)
-
  const maybeAutofocus = (node: HTMLTextAreaElement | null) => {
    if (!node) {


I'm a bit curious about how often this gets called - is it every render, or is it only when the ref (node) is changed?

fallaciousreasoning · 2024-09-16T07:42:08Z

browser/ui/webui/ai_chat/ai_chat_ui.cc

+  content::WebContents* chat_web_contents = GetChatWebContents();
+  size_t visible_conversation_history_size = 0;
+  if (chat_web_contents) {
+    ai_chat::AIChatTabHelper* active_chat_tab_helper =
+        ai_chat::AIChatTabHelper::FromWebContents(chat_web_contents);
+    visible_conversation_history_size =
+        active_chat_tab_helper->GetVisibleConversationHistory().size();
+  }
+  untrusted_source->AddBoolean("hasInitialHistory",
+                               visible_conversation_history_size != 0);


I think @petemill means here where we call getConversationHistory

If its the first call, and we get nothing back, we don't have initial history.

brave-builds · 2024-10-03T15:35:38Z

A Storybook has been deployed to preview UI for the latest push

brave-builds · 2025-01-17T22:07:31Z

A Storybook has been deployed to preview UI for the latest push

SergeyZhukovsky requested review from petemill and fallaciousreasoning September 10, 2024 19:45

SergeyZhukovsky self-assigned this Sep 10, 2024

github-actions bot added the CI/storybook-url Deploy storybook and provide a unique URL for each build label Sep 10, 2024

SergeyZhukovsky force-pushed the android_leo_input_box_focus branch 2 times, most recently from 9151f4f to 00844ae Compare September 10, 2024 22:15

fallaciousreasoning reviewed Sep 11, 2024

View reviewed changes

SergeyZhukovsky force-pushed the android_leo_input_box_focus branch from 6932add to 4756a38 Compare September 11, 2024 13:48

petemill reviewed Sep 11, 2024

View reviewed changes

SergeyZhukovsky force-pushed the android_leo_input_box_focus branch 2 times, most recently from d81551d to 42164ae Compare September 12, 2024 18:22

fallaciousreasoning approved these changes Sep 16, 2024

View reviewed changes

SergeyZhukovsky force-pushed the android_leo_input_box_focus branch from 42164ae to 07cab19 Compare October 2, 2024 18:53

github-actions bot added the needs-security-review label Oct 2, 2024

github-actions bot assigned thypon Oct 2, 2024

SergeyZhukovsky force-pushed the android_leo_input_box_focus branch 2 times, most recently from b954367 to 39a8cdd Compare October 3, 2024 14:50

thypon removed the needs-security-review label Oct 3, 2024

thypon removed their assignment Oct 3, 2024

SergeyZhukovsky force-pushed the android_leo_input_box_focus branch from 39a8cdd to 0cdd853 Compare January 17, 2025 18:35

Autofocuses input box on Android Leo chat window on a fresh chat

36292c7

SergeyZhukovsky force-pushed the android_leo_input_box_focus branch from 0cdd853 to 36292c7 Compare January 17, 2025 21:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autofocuses input box on Android Leo chat window on a fresh chat #25512

Autofocuses input box on Android Leo chat window on a fresh chat #25512

SergeyZhukovsky commented Sep 10, 2024 •

edited

Loading

brave-builds commented Sep 10, 2024

brave-builds commented Sep 10, 2024

brave-builds commented Sep 10, 2024

fallaciousreasoning left a comment

fallaciousreasoning Sep 11, 2024

fallaciousreasoning Sep 11, 2024

fallaciousreasoning Sep 11, 2024

fallaciousreasoning Sep 11, 2024

SergeyZhukovsky commented Sep 11, 2024

petemill Sep 11, 2024

SergeyZhukovsky Sep 12, 2024

fallaciousreasoning Sep 16, 2024

SergeyZhukovsky Sep 16, 2024

fallaciousreasoning Sep 17, 2024

petemill Sep 11, 2024

SergeyZhukovsky Sep 12, 2024 •

edited

Loading

SergeyZhukovsky Sep 12, 2024

petemill left a comment

SergeyZhukovsky commented Sep 12, 2024 •

edited

Loading

SergeyZhukovsky commented Sep 13, 2024

fallaciousreasoning left a comment

fallaciousreasoning Sep 16, 2024

fallaciousreasoning Sep 16, 2024

fallaciousreasoning Sep 16, 2024

fallaciousreasoning Sep 16, 2024

fallaciousreasoning Sep 16, 2024

brave-builds commented Oct 3, 2024

brave-builds commented Jan 17, 2025

	handleShowSoftKeyboard?: () => unknown
	handleShowSoftKeyboard?: () => void

Autofocuses input box on Android Leo chat window on a fresh chat #25512

Are you sure you want to change the base?

Autofocuses input box on Android Leo chat window on a fresh chat #25512

Conversation

SergeyZhukovsky commented Sep 10, 2024 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

brave-builds commented Sep 10, 2024

brave-builds commented Sep 10, 2024

brave-builds commented Sep 10, 2024

fallaciousreasoning left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyZhukovsky commented Sep 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyZhukovsky Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petemill left a comment

Choose a reason for hiding this comment

SergeyZhukovsky commented Sep 12, 2024 • edited Loading

SergeyZhukovsky commented Sep 13, 2024

fallaciousreasoning left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brave-builds commented Oct 3, 2024

brave-builds commented Jan 17, 2025

SergeyZhukovsky commented Sep 10, 2024 •

edited

Loading

SergeyZhukovsky Sep 12, 2024 •

edited

Loading

SergeyZhukovsky commented Sep 12, 2024 •

edited

Loading