Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AI Chat fullpage UI notices and polish #26678

Merged
merged 27 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a0cd28b
AIChat storage notice and more reliable initial state of agreements a…
petemill Nov 20, 2024
2e100aa
More reliable determination if standalone or not
petemill Nov 21, 2024
2e227da
Optimize loading by putting global variable fetching in non-react code
petemill Nov 21, 2024
015a5f0
conversation-list empty notice and whole page loading indicator (and …
petemill Nov 21, 2024
7b25136
fix ios
petemill Nov 21, 2024
cec9b01
Notice for enabling storage pref
petemill Nov 22, 2024
b0ae703
Full Page collapsed nav shouldn't show new conversation button if con…
petemill Nov 22, 2024
181775c
notice polish
petemill Nov 23, 2024
0a7a34f
AI Chat conversation items get their intended max-width and line up w…
petemill Nov 23, 2024
c46cbec
adjust strings based on review
petemill Nov 23, 2024
aed5f28
global state cleanup attempt based on review
petemill Nov 26, 2024
6ab6965
nicer notice close button style
petemill Nov 27, 2024
2af7db1
Fix storybook so its not always non-tab-associated conversation
petemill Nov 27, 2024
bbd9aa1
fix conversation title back button wrapping
petemill Nov 27, 2024
6c84eb7
fix import order
petemill Nov 27, 2024
5797007
fix unit test
petemill Nov 27, 2024
730eebf
formatting
petemill Nov 27, 2024
d1def02
navigation width constant through animation
petemill Nov 27, 2024
abc00df
spacing variables
petemill Nov 27, 2024
cedc3c6
don't use ref for useeffect
petemill Nov 27, 2024
f37b875
nala space variables for notice
petemill Nov 27, 2024
4b6d843
actually subscribe to state change!
petemill Nov 27, 2024
2bc3ba2
remove duplicate string
petemill Nov 27, 2024
bff2e31
fix storage notice coming back because api state wasn't updated
petemill Nov 27, 2024
edf9058
fix css variable
petemill Nov 27, 2024
922bd94
fix ios
petemill Nov 27, 2024
e868e42
iOS gets canShowPremiumPrompt
petemill Nov 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 13 additions & 19 deletions browser/ui/webui/ai_chat/ai_chat_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "brave/components/constants/webui_url_constants.h"
#include "brave/components/l10n/common/localization_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/tabs/tab_model.h"
#include "chrome/browser/ui/webui/webui_util.h"
#include "components/grit/brave_components_resources.h"
#include "components/prefs/pref_service.h"
Expand Down Expand Up @@ -79,13 +80,6 @@ AIChatUI::AIChatUI(content::WebUI* web_ui)
str.name, brave_l10n::GetLocalizedResourceUTF16String(str.id));
}

base::Time last_accepted_disclaimer =
profile_->GetOriginalProfile()->GetPrefs()->GetTime(
ai_chat::prefs::kLastAcceptedDisclaimer);

untrusted_source->AddBoolean("hasAcceptedAgreement",
!last_accepted_disclaimer.is_null());

#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS)
constexpr bool kIsMobile = true;
#else
Expand All @@ -96,11 +90,6 @@ AIChatUI::AIChatUI(content::WebUI* web_ui)
untrusted_source->AddBoolean("isHistoryEnabled",
ai_chat::features::IsAIChatHistoryEnabled());

untrusted_source->AddBoolean(
"hasUserDismissedPremiumPrompt",
profile_->GetOriginalProfile()->GetPrefs()->GetBoolean(
ai_chat::prefs::kUserDismissedPremiumPrompt));

untrusted_source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::ScriptSrc,
"script-src 'self' chrome-untrusted://resources;");
Expand Down Expand Up @@ -128,21 +117,26 @@ void AIChatUI::BindInterface(
if (embedder_) {
embedder_->ShowUI();
}

// Get the WebContents which SidePanel mode should be associated with
content::WebContents* web_contents = nullptr;
#if !BUILDFLAG(IS_ANDROID)
Browser* browser =
ai_chat::GetBrowserForWebContents(web_ui()->GetWebContents());
if (!browser) {
return;
if (browser) {
TabStripModel* tab_strip_model = browser->tab_strip_model();
if (tab_strip_model) {
// If this WebUI is a main tab, we never want to be associated with
// the active tab
if (tab_strip_model->GetIndexOfWebContents(web_ui()->GetWebContents()) ==
TabStripModel::kNoTab) {
web_contents = tab_strip_model->GetActiveWebContents();
}
}
}

TabStripModel* tab_strip_model = browser->tab_strip_model();
DCHECK(tab_strip_model);
web_contents = tab_strip_model->GetActiveWebContents();
#else
web_contents = GetActiveWebContents(profile_);
#endif
// Don't associate with the WebUI's WebContents
if (web_contents == web_ui()->GetWebContents()) {
web_contents = nullptr;
}
Expand Down
6 changes: 3 additions & 3 deletions browser/ui/webui/ai_chat/ai_chat_ui_page_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,10 @@ void AIChatUIPageHandler::CloseUI() {
#endif
}

void AIChatUIPageHandler::SetChatUI(
mojo::PendingRemote<mojom::ChatUI> chat_ui) {
void AIChatUIPageHandler::SetChatUI(mojo::PendingRemote<mojom::ChatUI> chat_ui,
SetChatUICallback callback) {
chat_ui_.Bind(std::move(chat_ui));
chat_ui_->SetInitialData(active_chat_tab_helper_ == nullptr);
std::move(callback).Run(active_chat_tab_helper_ == nullptr);
}

void AIChatUIPageHandler::BindRelatedConversation(
Expand Down
3 changes: 2 additions & 1 deletion browser/ui/webui/ai_chat/ai_chat_ui_page_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ class AIChatUIPageHandler : public mojom::AIChatUIHandler,
void ManagePremium() override;
void HandleVoiceRecognition(const std::string& conversation_uuid) override;
void CloseUI() override;
void SetChatUI(mojo::PendingRemote<mojom::ChatUI> chat_ui) override;
void SetChatUI(mojo::PendingRemote<mojom::ChatUI> chat_ui,
SetChatUICallback callback) override;
void BindRelatedConversation(
mojo::PendingReceiver<mojom::ConversationHandler> receiver,
mojo::PendingRemote<mojom::ConversationUI> conversation_ui_handler)
Expand Down
92 changes: 53 additions & 39 deletions components/ai_chat/core/browser/ai_chat_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ AIChatService::AIChatService(
prefs::kStorageEnabled,
base::BindRepeating(&AIChatService::MaybeInitStorage,
weak_ptr_factory_.GetWeakPtr()));
pref_change_registrar_.Add(
prefs::kUserDismissedPremiumPrompt,
base::BindRepeating(&AIChatService::OnStateChanged,
weak_ptr_factory_.GetWeakPtr()));

MaybeInitStorage();
}
Expand Down Expand Up @@ -362,6 +366,7 @@ void AIChatService::MaybeInitStorage() {
weak_ptr_factory_.GetWeakPtr()));
}
}
OnStateChanged();
}

void AIChatService::OnOsCryptAsyncReady(os_crypt_async::Encryptor encryptor,
Expand Down Expand Up @@ -548,6 +553,18 @@ void AIChatService::MarkAgreementAccepted() {
SetUserOptedIn(profile_prefs_, true);
}

void AIChatService::EnableStoragePref() {
profile_prefs_->SetBoolean(prefs::kStorageEnabled, true);
}

void AIChatService::DismissStorageNotice() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We reach this if the user presses X on the top-right of the Conversation history popup in Leo, right? We also need an explicit "Don't do this" or "No thanks" button on that popup, otherwise this seems like a consent dark pattern. The language on that popup right now makes me think that the X will actually just dismiss the popup without actually opting me out of this feature change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShivanKaul The storage notice is dismissed either by clicking the X or after it's been viewed once for a few seconds.
As far as I understand, the opinion was that we did not need to present the opt-out preference inline as it's not going to affect any previous actions the user does, only the future actions. And that we don't need explicit opt-in permission from the user to store submitted conversations on their own computer (encrypted) for the AI chat feature. This seems more of a notice about the new feature. @mattmcalister

profile_prefs_->SetBoolean(prefs::kUserDismissedStorageNotice, true);
}

void AIChatService::DismissPremiumPrompt() {
profile_prefs_->SetBoolean(prefs::kUserDismissedPremiumPrompt, true);
}

void AIChatService::GetActionMenuList(GetActionMenuListCallback callback) {
std::move(callback).Run(ai_chat::GetActionMenuList());
}
Expand All @@ -558,41 +575,6 @@ void AIChatService::GetPremiumStatus(GetPremiumStatusCallback callback) {
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}

void AIChatService::GetCanShowPremiumPrompt(
GetCanShowPremiumPromptCallback callback) {
bool has_user_dismissed_prompt =
profile_prefs_->GetBoolean(prefs::kUserDismissedPremiumPrompt);

if (has_user_dismissed_prompt) {
std::move(callback).Run(false);
return;
}

base::Time last_accepted_disclaimer =
profile_prefs_->GetTime(prefs::kLastAcceptedDisclaimer);

// Can't show if we haven't accepted disclaimer yet
if (last_accepted_disclaimer.is_null()) {
std::move(callback).Run(false);
return;
}

base::Time time_1_day_ago = base::Time::Now() - base::Days(1);
bool is_more_than_24h_since_last_seen =
last_accepted_disclaimer < time_1_day_ago;

if (is_more_than_24h_since_last_seen) {
std::move(callback).Run(true);
return;
}

std::move(callback).Run(false);
}

void AIChatService::DismissPremiumPrompt() {
profile_prefs_->SetBoolean(prefs::kUserDismissedPremiumPrompt, true);
}

void AIChatService::DeleteConversation(const std::string& id) {
auto handler_it = conversation_handlers_.find(id);
if (handler_it != conversation_handlers_.end()) {
Expand Down Expand Up @@ -681,6 +663,38 @@ void AIChatService::MaybeUnloadConversation(
}
}

mojom::ServiceStatePtr AIChatService::BuildState() {
bool has_user_dismissed_storage_notice =
profile_prefs_->GetBoolean(prefs::kUserDismissedStorageNotice);
base::Time last_accepted_disclaimer =
profile_prefs_->GetTime(ai_chat::prefs::kLastAcceptedDisclaimer);

bool is_user_opted_in = !last_accepted_disclaimer.is_null();

// Premium prompt is only shown conditionally (e.g. the user hasn't dismissed
// it and it's been some time since the user started using the feature).
bool can_show_premium_prompt =
!profile_prefs_->GetBoolean(prefs::kUserDismissedPremiumPrompt) &&
!last_accepted_disclaimer.is_null() &&
last_accepted_disclaimer < base::Time::Now() - base::Days(1);

bool is_storage_enabled = profile_prefs_->GetBoolean(prefs::kStorageEnabled);

mojom::ServiceStatePtr state = mojom::ServiceState::New();
state->has_accepted_agreement = is_user_opted_in;
state->is_storage_pref_enabled = is_storage_enabled;
state->is_storage_notice_dismissed = has_user_dismissed_storage_notice;
state->can_show_premium_prompt = can_show_premium_prompt;
return state;
}

void AIChatService::OnStateChanged() {
mojom::ServiceStatePtr state = BuildState();
for (auto& remote : observer_remotes_) {
remote->OnStateChanged(state.Clone());
}
}

bool AIChatService::IsAIChatHistoryEnabled() {
return (features::IsAIChatHistoryEnabled() &&
profile_prefs_->GetBoolean(prefs::kStorageEnabled));
Expand Down Expand Up @@ -843,8 +857,10 @@ void AIChatService::BindConversation(
}

void AIChatService::BindObserver(
mojo::PendingRemote<mojom::ServiceObserver> observer) {
mojo::PendingRemote<mojom::ServiceObserver> observer,
BindObserverCallback callback) {
observer_remotes_.Add(std::move(observer));
std::move(callback).Run(BuildState());
}

bool AIChatService::HasUserOptedIn() {
Expand All @@ -866,16 +882,14 @@ size_t AIChatService::GetInMemoryConversationCountForTesting() {
}

void AIChatService::OnUserOptedIn() {
OnStateChanged();
bool is_opted_in = HasUserOptedIn();
if (!is_opted_in) {
return;
}
for (auto& kv : conversation_handlers_) {
kv.second->OnUserOptedIn();
}
for (auto& remote : observer_remotes_) {
remote->OnAgreementAccepted();
}
if (ai_chat_metrics_ != nullptr) {
ai_chat_metrics_->RecordEnabled(true, true, {});
}
Expand Down
11 changes: 7 additions & 4 deletions components/ai_chat/core/browser/ai_chat_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,13 @@ class AIChatService : public KeyedService,

// mojom::Service
void MarkAgreementAccepted() override;
void EnableStoragePref() override;
void DismissStorageNotice() override;
void DismissPremiumPrompt() override;
void GetVisibleConversations(
GetVisibleConversationsCallback callback) override;
void GetActionMenuList(GetActionMenuListCallback callback) override;
void GetPremiumStatus(GetPremiumStatusCallback callback) override;
void GetCanShowPremiumPrompt(
GetCanShowPremiumPromptCallback callback) override;
void DismissPremiumPrompt() override;
void DeleteConversation(const std::string& id) override;
void RenameConversation(const std::string& id,
const std::string& new_name) override;
Expand All @@ -152,7 +152,8 @@ class AIChatService : public KeyedService,
mojo::PendingReceiver<mojom::ConversationHandler> receiver,
mojo::PendingRemote<mojom::ConversationUI> conversation_ui_handler)
override;
void BindObserver(mojo::PendingRemote<mojom::ServiceObserver> ui) override;
void BindObserver(mojo::PendingRemote<mojom::ServiceObserver> ui,
BindObserverCallback callback) override;

bool HasUserOptedIn();
bool IsPremiumStatus();
Expand Down Expand Up @@ -208,6 +209,8 @@ class AIChatService : public KeyedService,
mojom::PremiumStatus status,
mojom::PremiumInfoPtr info);
void OnDataDeletedForDisabledStorage(bool success);
mojom::ServiceStatePtr BuildState();
void OnStateChanged();

bool IsAIChatHistoryEnabled();

Expand Down
7 changes: 4 additions & 3 deletions components/ai_chat/core/browser/ai_chat_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "base/files/scoped_temp_dir.h"
#include "base/functional/bind.h"
#include "base/functional/callback.h"
#include "base/functional/callback_helpers.h"
#include "base/functional/overloaded.h"
#include "base/memory/scoped_refptr.h"
#include "base/run_loop.h"
Expand Down Expand Up @@ -72,8 +73,8 @@ class MockAIChatCredentialManager : public AIChatCredentialManager {
class MockServiceClient : public mojom::ServiceObserver {
public:
explicit MockServiceClient(AIChatService* service) {
service->BindObserver(
service_observer_receiver_.BindNewPipeAndPassRemote());
service->BindObserver(service_observer_receiver_.BindNewPipeAndPassRemote(),
base::DoNothing());
fallaciousreasoning marked this conversation as resolved.
Show resolved Hide resolved
service->Bind(service_remote_.BindNewPipeAndPassReceiver());
}

Expand All @@ -91,7 +92,7 @@ class MockServiceClient : public mojom::ServiceObserver {
(std::vector<mojom::ConversationPtr>),
(override));

MOCK_METHOD(void, OnAgreementAccepted, (), (override));
MOCK_METHOD(void, OnStateChanged, (mojom::ServiceStatePtr), (override));

private:
mojo::Receiver<mojom::ServiceObserver> service_observer_receiver_{this};
Expand Down
13 changes: 13 additions & 0 deletions components/ai_chat/core/browser/constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,17 @@ base::span<const webui::LocalizedString> GetLocalizedStrings() {
{"searchInProgress", IDS_CHAT_UI_SEARCH_IN_PROGRESS},
{"searchQueries", IDS_CHAT_UI_SEARCH_QUERIES},
{"learnMore", IDS_CHAT_UI_LEARN_MORE},
{"closeNotice", IDS_CHAT_UI_CLOSE_NOTICE},
{"noticeConversationHistoryBody",
IDS_CHAT_UI_NOTICE_CONVERSATION_HISTORY_BODY},
{"noticeConversationHistoryEmpty",
IDS_CHAT_UI_NOTICE_CONVERSATION_HISTORY_EMPTY},
{"noticeConversationHistoryTitleDisabledPref",
IDS_CHAT_UI_NOTICE_CONVERSATION_HISTORY_TITLE_DISABLED_PREF},
{"noticeConversationHistoryDisabledPref",
IDS_CHAT_UI_NOTICE_CONVERSATION_HISTORY_DISABLED_PREF},
{"noticeConversationHistoryDisabledPrefButton",
IDS_CHAT_UI_NOTICE_CONVERSATION_HISTORY_DISABLED_PREF_BUTTON},
{"leoSettingsTooltipLabel", IDS_CHAT_UI_LEO_SETTINGS_TOOLTIP_LABEL},
{"summarizePageButtonLabel", IDS_CHAT_UI_SUMMARIZE_PAGE},
{"welcomeGuideTitle", IDS_CHAT_UI_WELCOME_GUIDE_TITLE},
Expand Down Expand Up @@ -171,6 +182,8 @@ base::span<const webui::LocalizedString> GetLocalizedStrings() {
{"useMicButtonLabel", IDS_AI_CHAT_USE_MICROPHONE_BUTTON_LABEL},
{"menuTitleCustomModels", IDS_AI_CHAT_MENU_TITLE_CUSTOM_MODELS},
{"startConversationLabel", IDS_AI_CHAT_START_CONVERSATION_LABEL},
{"goBackToActiveConversationButton",
IDS_AI_CHAT_GO_BACK_TO_ACTIVE_CONVERSATION_BUTTON},
{"conversationListUntitled", IDS_AI_CHAT_CONVERSATION_LIST_UNTITLED}});

return kLocalizedStrings;
Expand Down
Loading
Loading