Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
petemill committed Dec 10, 2024
1 parent ed808cd commit 7f157d8
Show file tree
Hide file tree
Showing 22 changed files with 72 additions and 81 deletions.
15 changes: 4 additions & 11 deletions browser/ai_chat/ai_chat_throttle_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,8 @@
namespace ai_chat {

namespace {
constexpr char kTestProfileName[] = "TestProfile";

GURL GetAIChatUIURL() {
return GURL(kAIChatUIURL);
}

GURL GetAIChatUntrustedConversationUIURL() {
return GURL(kAIChatUntrustedConversationUIURL);
}
constexpr char kTestProfileName[] = "TestProfile";

} // namespace

Expand Down Expand Up @@ -84,7 +77,7 @@ INSTANTIATE_TEST_SUITE_P(
TEST_P(AiChatThrottleUnitTest, CancelNavigationFromTab) {
content::MockNavigationHandle test_handle(web_contents());

test_handle.set_url(GetAIChatUIURL());
test_handle.set_url(GURL(kAIChatUIURL)());

#if BUILDFLAG(IS_ANDROID)
ui::PageTransition transition = ui::PageTransitionFromInt(
Expand Down Expand Up @@ -112,7 +105,7 @@ TEST_P(AiChatThrottleUnitTest, CancelNavigationFromTab) {
TEST_P(AiChatThrottleUnitTest, CancelNavigationToFrame) {
content::MockNavigationHandle test_handle(web_contents());

test_handle.set_url(GetAIChatUntrustedConversationUIURL());
test_handle.set_url(GURL(kAIChatUntrustedConversationUIURL)());

#if BUILDFLAG(IS_ANDROID)
ui::PageTransition transition = ui::PageTransitionFromInt(
Expand All @@ -135,7 +128,7 @@ TEST_P(AiChatThrottleUnitTest, CancelNavigationToFrame) {
TEST_P(AiChatThrottleUnitTest, AllowNavigationFromPanel) {
content::MockNavigationHandle test_handle(web_contents());

test_handle.set_url(GetAIChatUIURL());
test_handle.set_url(GURL(kAIChatUIURL)());

#if BUILDFLAG(IS_ANDROID)
ui::PageTransition transition =
Expand Down
9 changes: 2 additions & 7 deletions browser/ui/webui/ai_chat/ai_chat_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,7 @@ AIChatUI::AIChatUI(content::WebUI* web_ui)
brave_l10n::GetLocalizedResourceUTF16String(str.id));
}

#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS)
constexpr bool kIsMobile = true;
#else
constexpr bool kIsMobile = false;
#endif

constexpr bool kIsMobile = BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS);
source->AddBoolean("isMobile", kIsMobile);
source->AddBoolean("isHistoryEnabled",
ai_chat::features::IsAIChatHistoryEnabled());
Expand All @@ -101,7 +96,7 @@ AIChatUI::AIChatUI(content::WebUI* web_ui)
"img-src 'self' blob: chrome://resources chrome://favicon2;");
source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::FontSrc,
"font-src 'self' data: chrome://resources;");
"font-src 'self' chrome://resources;");
source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::ChildSrc,
base::StringPrintf("child-src %s;", kAIChatUntrustedConversationUIURL));
Expand Down
9 changes: 3 additions & 6 deletions browser/ui/webui/ai_chat/ai_chat_untrusted_conversation_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include "brave/components/ai_chat/resources/grit/ai_chat_ui_generated_map.h"
#include "brave/components/constants/webui_url_constants.h"
#include "brave/components/l10n/common/localization_util.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/webui/webui_util.h"
#include "components/grit/brave_components_resources.h"
#include "content/public/browser/web_contents.h"
Expand All @@ -29,6 +28,8 @@

#if BUILDFLAG(IS_ANDROID)
#include "brave/browser/ui/android/ai_chat/brave_leo_settings_launcher_helper.h"
#else
#include "chrome/browser/ui/browser.h"
#endif

namespace {
Expand Down Expand Up @@ -137,11 +138,7 @@ AIChatUntrustedConversationUI::AIChatUntrustedConversationUI(
brave_l10n::GetLocalizedResourceUTF16String(str.id));
}

#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS)
constexpr bool kIsMobile = true;
#else
constexpr bool kIsMobile = false;
#endif
constexpr bool kIsMobile = BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS);
source->AddBoolean("isMobile", kIsMobile);

source->OverrideContentSecurityPolicy(
Expand Down
14 changes: 9 additions & 5 deletions components/ai_chat/core/browser/conversation_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@
#include "brave/components/ai_chat/core/browser/types.h"
#include "brave/components/ai_chat/core/browser/utils.h"
#include "brave/components/ai_chat/core/common/features.h"
#include "brave/components/ai_chat/core/common/mojom/ai_chat.mojom-forward.h"
#include "brave/components/ai_chat/core/common/mojom/ai_chat.mojom-shared.h"
#include "brave/components/ai_chat/core/common/mojom/ai_chat.mojom.h"
#include "brave/components/api_request_helper/api_request_helper.h"
#include "components/grit/brave_components_strings.h"
Expand Down Expand Up @@ -544,8 +542,14 @@ void ConversationHandler::RateMessage(bool is_liked,
base::BindOnce(
[](RateMessageCallback callback, APIRequestResult result) {
if (result.Is2XXResponseCode() && result.value_body().is_dict()) {
std::string id = *result.value_body().GetDict().FindString("id");
std::move(callback).Run(id);
const std::string* id_result =
result.value_body().GetDict().FindString("id");
if (id_result) {
std::move(callback).Run(*id_result);
} else {
DLOG(ERROR) << "Failed to get rating ID";
std::move(callback).Run(std::nullopt);
}
return;
}
DLOG(ERROR) << "Failed to send rating: " << result.response_code();
Expand Down Expand Up @@ -1636,7 +1640,7 @@ ConversationHandler::GetStateForConversationEntries() {
entries_state->is_content_refined = is_content_refined_;
entries_state->is_leo_model = is_leo_model;
entries_state->content_used_percentage =
(metadata_->associated_content->is_content_association_possible == true)
metadata_->associated_content->is_content_association_possible
? std::make_optional(
metadata_->associated_content->content_used_percentage)
: std::nullopt;
Expand Down
10 changes: 6 additions & 4 deletions components/ai_chat/resources/common/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,25 @@ export default abstract class API<T> {
this.state = {...defaultState}
}

// Debounce multiple state changes in the same task causing multiple
// events to be dispatched.
private dispatchDebouncedStateChange = debounce(() => {
console.debug('dispatching uistatechange event', {...this.state})
this.eventTarget.dispatchEvent(
new Event('uistatechange')
new Event('statechange')
)
}, 0)

public setPartialState(partialState: Partial<T>) {
console.log('setPartialState', partialState)
this.state = { ...this.state, ...partialState }
this.dispatchDebouncedStateChange()
}

public addStateChangeListener(callback: (event: Event) => void) {
this.eventTarget.addEventListener('uistatechange', callback)
this.eventTarget.addEventListener('statechange', callback)
}

public removeStateChangeListener(callback: (event: Event) => void) {
this.eventTarget.removeEventListener('uistatechange', callback)
this.eventTarget.removeEventListener('statechange', callback)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

import * as React from 'react'
import Button from '@brave/leo/react/button'
import Icon from '@brave/leo/react/icon'
import * as Mojom from '../../../common/mojom'
import styles from './style.module.scss'
import * as React from 'react'
import { getLocale } from '$web-common/locale'
import Button from '@brave/leo/react/button'
import * as Mojom from '../../mojom'
import styles from './style.module.scss'

interface ActionTypeLabelProps {
actionType: Mojom.ActionType
Expand Down
8 changes: 4 additions & 4 deletions components/ai_chat/resources/common/useAPIState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@
import * as React from 'react'
import API from './api'

export default function useAPIState<T, Y>(api: API<Y>, defaultState: T) {
export default function useAPIState<T>(api: API<T>, defaultState: T) {
// Intialize with global state that may have been set between module-load
// time and the first React render.
const [context, setContext] = React.useState<T>({
...defaultState,
...api.state
})
const updateFromAPIState = (state: Y) => {
const updateFromAPIState = React.useCallback((state: T) => {
setContext((value) => ({
...value,
...state
}))
}
}, [])

React.useEffect(() => {
// Update with any global state change that may have occurred between
Expand All @@ -34,6 +34,6 @@ export default function useAPIState<T, Y>(api: API<Y>, defaultState: T) {
return () => {
api.removeStateChangeListener(onGlobalStateChange)
}
}, [])
}, [updateFromAPIState])
return context
}
5 changes: 3 additions & 2 deletions components/ai_chat/resources/page/ai_chat_ui.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
<link rel="stylesheet" href="//resources/brave/fonts/manrope.css">
<link rel="stylesheet" href="//resources/brave/fonts/poppins.css">
<link rel="stylesheet" href="//resources/brave/fonts/inter.css">
<link rel="stylesheet" href="//resources/brave/leo/css/variables.css">
<link rel="stylesheet" href="./styles.css">
<script src="//resources/js/load_time_data_deprecated.js"></script>
<script src="/strings.js"></script>
<link rel="stylesheet" href="./styles.css">
<script src="chat_ui.bundle.js" type="module"></script>
<link rel="icon" href="//resources/brave-icons/social-leo-favicon-fullheight-color.svg">
<style>
@media (prefers-color-scheme: dark) {
Expand All @@ -30,6 +32,5 @@
</head>
<body>
<div id="mountPoint"></div>
<script src="chat_ui.bundle.js"></script>
</body>
</html>
6 changes: 3 additions & 3 deletions components/ai_chat/resources/page/chat_ui.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import * as React from 'react'
import { createRoot } from 'react-dom/client'
import { setIconBasePath } from '@brave/leo/react/icon'
import '@brave/leo/tokens/css/variables.css'
import '$web-components/app.global.scss'
import '$web-common/defaultTrustedTypesPolicy'
import getAPI from './api'
Expand Down Expand Up @@ -57,7 +56,7 @@ function Content() {

function ConversationEntries(props: ConversationEntriesProps) {
const conversationContext = useConversation()
const iframeRef = React.useRef<HTMLIFrameElement|null>(null)
const iframeRef = React.useRef<HTMLIFrameElement | null>(null)
const hasNotifiedContentReady = React.useRef(false)
const [hasLoaded, setHasLoaded] = React.useState(false)

Expand Down Expand Up @@ -114,7 +113,8 @@ function ConversationEntries(props: ConversationEntriesProps) {

return (
<iframe
src={"chrome-untrusted://leo-ai-conversation-entries/" + conversationContext.conversationUuid}
sandbox='allow-scripts allow-same-origin'
src={'chrome-untrusted://leo-ai-conversation-entries/' + conversationContext.conversationUuid}
ref={iframeRef}
onLoad={() => setHasLoaded(true)}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ function FeedbackForm() {

const canSubmit = !!category

const handleCancelClick = () => {
conversationContext.handleFeedbackFormCancel()
}

const handleSubmit = () => {
conversationContext.handleFeedbackFormSubmit(category, feedbackText, shouldSendUrl)
}
Expand Down Expand Up @@ -115,7 +111,7 @@ function FeedbackForm() {
</div>
)}
<fieldset className={styles.actions}>
<Button onClick={handleCancelClick} kind='plain-faint'>
<Button onClick={conversationContext.handleFeedbackFormCancel} kind='plain-faint'>
{getLocale('cancelButtonLabel')}
</Button>
<Button isDisabled={!canSubmit} onClick={handleSubmit}>
Expand Down
10 changes: 4 additions & 6 deletions components/ai_chat/resources/page/components/input_box/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,15 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

import Icon from '@brave/leo/react/icon'
import Button from '@brave/leo/react/button'
import * as React from 'react'

import classnames from '$web-common/classnames'
import { getLocale } from '$web-common/locale'
import Icon from '@brave/leo/react/icon'
import Button from '@brave/leo/react/button'

import styles from './style.module.scss'
import ActionTypeLabel from '../../../common/components/action_type_label'
import { AIChatContext } from '../../state/ai_chat_context'
import { ConversationContext } from '../../state/conversation_context'
import ActionTypeLabel from '../../../untrusted_conversation_frame/components/action_type_label'
import styles from './style.module.scss'

type Props = Pick<
ConversationContext,
Expand Down
18 changes: 9 additions & 9 deletions components/ai_chat/resources/page/components/main/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ const SCROLL_BOTTOM_THRESHOLD = 20
// when automatically scrolling to bottom.
const SCROLL_BOTTOM_PADDING = 18

const SUGGESTION_STATUS_SHOW_BUTTON: Mojom.SuggestionGenerationStatus[] = [
const SUGGESTION_STATUS_SHOW_BUTTON = new Set<Mojom.SuggestionGenerationStatus>([
Mojom.SuggestionGenerationStatus.CanGenerate,
Mojom.SuggestionGenerationStatus.IsGenerating
]
])

function Main() {
const aiChatContext = useAIChat()
Expand Down Expand Up @@ -144,7 +144,7 @@ function Main() {
const showSuggestions: boolean =
aiChatContext.hasAcceptedAgreement &&
(conversationContext.suggestedQuestions.length > 0 ||
SUGGESTION_STATUS_SHOW_BUTTON.includes(conversationContext.suggestionStatus))
SUGGESTION_STATUS_SHOW_BUTTON.has(conversationContext.suggestionStatus))

const viewPortWithoutKeyboard = React.useRef(0)
const keyboardSize = React.useRef(0)
Expand Down Expand Up @@ -246,11 +246,11 @@ function Main() {

<div ref={scrollAnchor}>
{!!conversationContext.conversationUuid &&
<aiChatContext.conversationEntriesComponent
onIsContentReady={setIsContentReady}
onHeightChanged={handleConversationEntriesHeightChanged}
/>
}
<aiChatContext.conversationEntriesComponent
onIsContentReady={setIsContentReady}
onHeightChanged={handleConversationEntriesHeightChanged}
/>
}
</div>

{conversationContext.isFeedbackFormVisible &&
Expand All @@ -263,7 +263,7 @@ function Main() {
<div className={styles.suggestionsContainer}>
<div className={styles.questionsList}>
{conversationContext.suggestedQuestions.map((question, i) => <SuggestedQuestion key={question} question={question} />)}
{SUGGESTION_STATUS_SHOW_BUTTON.includes(
{SUGGESTION_STATUS_SHOW_BUTTON.has(
conversationContext.suggestionStatus
) && conversationContext.shouldSendPageContents && (
<GenerateSuggestionsButton />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@

&.contentReady {
opacity: 1;
transition: opacity 0.125s ease-out;
transition: opacity 0.12s ease-out;
}
}

Expand Down Expand Up @@ -178,7 +178,7 @@
}

.questionsList {
--leo-button-radius: 12px;
--leo-button-radius: var(--leo-radius-l);
display: flex;
flex-direction: column;
align-items: start;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ import * as Mojom from '../../../common/mojom'
import { useAIChat } from '../../state/ai_chat_context'
import { useConversation } from '../../state/conversation_context'
import styles from './style.module.scss'
import { getKeysForMojomEnum } from '$web-common/mojomUtils'

function getCategoryName(category: Mojom.ModelCategory) {
// To avoid problems when order of enum values change, we base the key
// on the enum name rather than the number value, e.g. "CHAT" vs 0
const categoryKey = Object.keys(Mojom.ModelCategory)[category]
const categoryKey = getKeysForMojomEnum(Mojom.ModelCategory)[category]
const key = `modelCategory-${categoryKey.toLowerCase()}`
return getLocale(key)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export function GenerateSuggestionsButton() {
const conversationContext = useConversation()
return (
<SuggestionButton
onClick={() => conversationContext.generateSuggestedQuestions()}
onClick={conversationContext.generateSuggestedQuestions}
isLoading={conversationContext.suggestionStatus === Mojom.SuggestionGenerationStatus.IsGenerating}
>
<span className={styles.generateButtonText}>
Expand Down
4 changes: 2 additions & 2 deletions components/ai_chat/resources/page/state/ai_chat_context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import * as Mojom from '../../common/mojom'
import getAPI, * as AIChat from '../api'

export interface ConversationEntriesProps {
onIsContentReady: (isContentReady: boolean) => unknown
onHeightChanged: () => unknown
onIsContentReady: (isContentReady: boolean) => void
onHeightChanged: () => void
}

type AIChatContextProps = {
Expand Down
Loading

0 comments on commit 7f157d8

Please sign in to comment.