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 1 commit
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
4 changes: 4 additions & 0 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
10 changes: 7 additions & 3 deletions ios/brave-ios/Sources/AIChat/ModelView/AIChatViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class AIChatViewModel: NSObject, ObservableObject {

@Published var siteInfo: AiChat.SiteInfo?
@Published var _shouldSendPageContents: Bool = true
@Published var canShowPremiumPrompt: Bool = false
@Published var _canShowPremiumPrompt: Bool = false
@Published var premiumStatus: AiChat.PremiumStatus = .inactive
@Published var suggestedQuestions: [String] = []
@Published var suggestionsStatus: AiChat.SuggestionGenerationStatus = .none
Expand Down Expand Up @@ -68,11 +68,11 @@ public class AIChatViewModel: NSObject, ObservableObject {

public var shouldShowPremiumPrompt: Bool {
get {
return canShowPremiumPrompt
return _canShowPremiumPrompt
}

set { // swiftlint:disable:this unused_setter_value
self.canShowPremiumPrompt = newValue
_canShowPremiumPrompt = newValue
Copy link
Member Author

Choose a reason for hiding this comment

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

@Brandon-T we don't need to call objectWillChange.send() right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct. Not necessary.

if !newValue {
api.dismissPremiumPrompt()
}
Expand Down Expand Up @@ -333,4 +333,8 @@ extension AIChatViewModel: AIChatDelegate {
self.siteInfo = siteInfo
self._shouldSendPageContents = shouldSendPageContents
}

public func onServiceStateChanged(_ state: AiChat.ServiceState) {
self._canShowPremiumPrompt = state.canShowPremiumPrompt
}
}
6 changes: 4 additions & 2 deletions ios/browser/api/ai_chat/ai_chat.mm
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ - (instancetype)initWithProfileIOS:(ProfileIOS*)profile
current_content_ = std::make_unique<ai_chat::AssociatedContentDriverIOS>(
profile_->GetSharedURLLoaderFactory(), delegate);

conversation_client_ = std::make_unique<ai_chat::ConversationClient>(
service_.get(), delegate_);

[self createNewConversation];
}
return self;
Expand All @@ -78,8 +81,7 @@ - (void)dealloc {
- (void)createNewConversation {
current_conversation_ = service_->CreateConversationHandlerForContent(
current_content_->GetContentId(), current_content_->GetWeakPtr());
conversation_client_ = std::make_unique<ai_chat::ConversationClient>(
current_conversation_.get(), delegate_);
conversation_client_->ChangeConversation(current_conversation_.get());
}

- (bool)isAgreementAccepted {
Expand Down
1 change: 1 addition & 0 deletions ios/browser/api/ai_chat/ai_chat_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ OBJC_EXPORT
status:(AiChatSuggestionGenerationStatus)status;
- (void)onPageHasContent:(AiChatSiteInfo*)siteInfo
shouldSendContent:(bool)shouldSendContent;
- (void)onServiceStateChanged:(AiChatServiceState*)state;
@end

NS_ASSUME_NONNULL_END
Expand Down
23 changes: 18 additions & 5 deletions ios/browser/api/ai_chat/conversation_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,29 @@
#include <string>
#include <vector>

#include "base/memory/weak_ptr.h"
#include "brave/components/ai_chat/core/common/mojom/ai_chat.mojom.h"
#include "mojo/public/cpp/bindings/receiver.h"

@protocol AIChatDelegate;

namespace ai_chat {

class AIChatService;
class ConversationHandler;

// TODO(petemill): Have AIChatViewModel.swift (aka AIChatDelegate) implement
// mojom::ConversationUI and bind directly to ConversationHandler via ai_chat.mm
// so that this proxy isn't neccessary.
class ConversationClient : public mojom::ConversationUI {
// mojom::ConversationUI and mojom::ServiceObserver and bind directly to
// ConversationHandler and AIChatService via ai_chat.mm so that this proxy isn't
// neccessary.
class ConversationClient : public mojom::ConversationUI,
public mojom::ServiceObserver {
public:
ConversationClient(ConversationHandler* conversation,
id<AIChatDelegate> bridge);
ConversationClient(AIChatService* ai_chat_service, id<AIChatDelegate> bridge);
~ConversationClient() override;

void ChangeConversation(ConversationHandler* conversation);

protected:
// mojom::ConversationUI
void OnConversationHistoryUpdate() override;
Expand All @@ -43,11 +48,19 @@ class ConversationClient : public mojom::ConversationUI {
void OnFaviconImageDataChanged() override;
void OnConversationDeleted() override;

// mojom::ServiceObserver
void OnStateChanged(mojom::ServiceStatePtr state) override;
void OnConversationListChanged(
std::vector<mojom::ConversationPtr> conversations) override {}

private:
// The actual UI
__weak id<AIChatDelegate> bridge_;

mojo::Receiver<mojom::ConversationUI> receiver_{this};
mojo::Receiver<mojom::ServiceObserver> service_receiver_{this};

base::WeakPtrFactory<ConversationClient> weak_ptr_factory_{this};
};

} // namespace ai_chat
Expand Down
18 changes: 16 additions & 2 deletions ios/browser/api/ai_chat/conversation_client.mm
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "ai_chat.mojom.objc+private.h"
#include "base/strings/sys_string_conversions.h"
#include "brave/base/mac/conversions.h"
#include "brave/components/ai_chat/core/browser/ai_chat_service.h"
#include "brave/components/ai_chat/core/browser/conversation_handler.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"
Expand All @@ -16,14 +17,22 @@

namespace ai_chat {

ConversationClient::ConversationClient(ConversationHandler* conversation,
ConversationClient::ConversationClient(AIChatService* ai_chat_service,
id<AIChatDelegate> bridge)
: bridge_(bridge) {
conversation->Bind(receiver_.BindNewPipeAndPassRemote());
ai_chat_service->BindObserver(
service_receiver_.BindNewPipeAndPassRemote(),
base::BindOnce(&ConversationClient::OnStateChanged,
weak_ptr_factory_.GetWeakPtr()));
}

ConversationClient::~ConversationClient() = default;

void ConversationClient::ChangeConversation(ConversationHandler* conversation) {
receiver_.reset();
conversation->Bind(receiver_.BindNewPipeAndPassRemote());
}

// MARK: - mojom::ConversationUI

void ConversationClient::OnConversationHistoryUpdate() {
Expand Down Expand Up @@ -75,4 +84,9 @@
// allows deletion.
}

void ConversationClient::OnStateChanged(const mojom::ServiceStatePtr state) {
[bridge_ onServiceStateChanged:[[AiChatServiceState alloc]
initWithServiceStatePtr:state->Clone()]];
}

} // namespace ai_chat
Loading