Skip to content

Commit ad845bf

Browse files
ergunshDevtools-frontend LUCI CQ
authored andcommitted
[AiAssistance] Do not pass render method to view methods
Passing a complete `render` method to helper render methods provide the ability for helper methods to trigger a full re-render. However, this is counterintuitive and such re-rendering should be decided on the presenter side. We needed full `re-render` from views while handling side effect confirmation and we can trigger such re-render directly in the final callback on the step itself. Bug: 394005781 Change-Id: Ib997320957f010173db4267d0a53d1e1edb8b547 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6252786 Auto-Submit: Ergün Erdoğmuş <[email protected]> Commit-Queue: Ergün Erdoğmuş <[email protected]> Reviewed-by: Alex Rudenko <[email protected]> Reviewed-by: Samiya Caur <[email protected]>
1 parent 7ae5c7d commit ad845bf

File tree

2 files changed

+16
-26
lines changed

2 files changed

+16
-26
lines changed

front_end/panels/ai_assistance/AiAssistancePanel.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -966,7 +966,11 @@ export class AiAssistancePanel extends UI.Panel.Panel {
966966
step.isLoading = false;
967967
step.code ??= data.code;
968968
step.sideEffect = {
969-
onAnswer: data.confirm,
969+
onAnswer: (result: boolean) => {
970+
data.confirm(result);
971+
step.sideEffect = undefined;
972+
void this.doUpdate();
973+
},
970974
};
971975
commitStep();
972976
break;

front_end/panels/ai_assistance/components/ChatView.ts

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,6 @@ export class ChatView extends HTMLElement {
712712
onFeedbackSubmit: this.#props.onFeedbackSubmit,
713713
onLastAnswerMarkdownViewRef: this.#handleLastAnswerMarkdownViewRef,
714714
onMessageContainerRef: this.#handleMessageContainerRef,
715-
render: this.#render,
716715
})}
717716
${this.#props.isReadOnly
718717
? renderReadOnlySection({
@@ -860,13 +859,16 @@ function renderStepCode(step: Step): Lit.LitTemplate {
860859
// clang-format on
861860
}
862861

863-
function renderStepDetails({step, markdownRenderer, isLast, render}: {
862+
function renderStepDetails({
863+
step,
864+
markdownRenderer,
865+
isLast,
866+
}: {
864867
step: Step,
865868
markdownRenderer: MarkdownRendererWithCodeBlock,
866869
isLast: boolean,
867-
render: () => void,
868870
}): Lit.LitTemplate {
869-
const sideEffects = isLast && step.sideEffect ? renderSideEffectConfirmationUi(step, render) : Lit.nothing;
871+
const sideEffects = isLast && step.sideEffect ? renderSideEffectConfirmationUi(step) : Lit.nothing;
870872
const thought = step.thought ? html`<p>${renderTextAsMarkdown(step.thought, markdownRenderer)}</p>` : Lit.nothing;
871873

872874
// clang-format off
@@ -924,12 +926,11 @@ function renderStepBadge({step, isLoading, isLast}: {
924926
></devtools-icon>`;
925927
}
926928

927-
function renderStep({step, isLoading, markdownRenderer, isLast, render}: {
929+
function renderStep({step, isLoading, markdownRenderer, isLast}: {
928930
step: Step,
929931
isLoading: boolean,
930932
markdownRenderer: MarkdownRendererWithCodeBlock,
931933
isLast: boolean,
932-
render: () => void,
933934
}): Lit.LitTemplate {
934935
const stepClasses = Lit.Directives.classMap({
935936
step: true,
@@ -952,22 +953,16 @@ function renderStep({step, isLoading, markdownRenderer, isLast, render}: {
952953
></devtools-icon>
953954
</div>
954955
</summary>
955-
${renderStepDetails({step, markdownRenderer, isLast, render})}
956+
${renderStepDetails({step, markdownRenderer, isLast})}
956957
</details>`;
957958
// clang-format on
958959
}
959960

960-
function renderSideEffectConfirmationUi(step: Step, render: () => void): Lit.LitTemplate {
961+
function renderSideEffectConfirmationUi(step: Step): Lit.LitTemplate {
961962
if (!step.sideEffect) {
962963
return Lit.nothing;
963964
}
964965

965-
const sideEffectAction = (answer: boolean): void => {
966-
step.sideEffect?.onAnswer(answer);
967-
step.sideEffect = undefined;
968-
render();
969-
};
970-
971966
// clang-format off
972967
return html`<div
973968
class="side-effect-confirmation"
@@ -982,7 +977,7 @@ function renderSideEffectConfirmationUi(step: Step, render: () => void): Lit.Lit
982977
jslogContext: 'decline-execute-code',
983978
} as Buttons.Button.ButtonData
984979
}
985-
@click=${() => sideEffectAction(false)}
980+
@click=${() => step.sideEffect?.onAnswer(false)}
986981
>${lockedString(
987982
UIStringsNotTranslate.negativeSideEffectConfirmation,
988983
)}</devtools-button>
@@ -994,7 +989,7 @@ function renderSideEffectConfirmationUi(step: Step, render: () => void): Lit.Lit
994989
iconName: 'play',
995990
} as Buttons.Button.ButtonData
996991
}
997-
@click=${() => sideEffectAction(true)}
992+
@click=${() => step.sideEffect?.onAnswer(true)}
998993
>${
999994
lockedString(UIStringsNotTranslate.positiveSideEffectConfirmation)
1000995
}</devtools-button>
@@ -1036,7 +1031,6 @@ function renderChatMessage({
10361031
onSuggestionClick,
10371032
onFeedbackSubmit,
10381033
onLastAnswerMarkdownViewRef,
1039-
render
10401034
}: {
10411035
message: ChatMessage,
10421036
isLoading: boolean,
@@ -1048,7 +1042,6 @@ function renderChatMessage({
10481042
onSuggestionClick: (suggestion: string) => void,
10491043
onFeedbackSubmit: (rpcId: Host.AidaClient.RpcGlobalId, rate: Host.AidaClient.Rating, feedback?: string) => void,
10501044
onLastAnswerMarkdownViewRef: (el: Element|undefined) => void,
1051-
render: () => void,
10521045
}): Lit.TemplateResult {
10531046
if (message.entity === ChatMessageEntity.USER) {
10541047
const name = userInfo.accountFullName || lockedString(UIStringsNotTranslate.you);
@@ -1094,7 +1087,6 @@ function renderChatMessage({
10941087
isLoading,
10951088
markdownRenderer,
10961089
isLast: [...message.steps.values()].at(-1) === step && isLast,
1097-
render
10981090
});
10991091
},
11001092
)}
@@ -1203,7 +1195,6 @@ function renderMessages({
12031195
onFeedbackSubmit,
12041196
onLastAnswerMarkdownViewRef,
12051197
onMessageContainerRef,
1206-
render
12071198
}: {
12081199
messages: ChatMessage[],
12091200
isLoading: boolean,
@@ -1215,7 +1206,6 @@ function renderMessages({
12151206
onFeedbackSubmit: (rpcId: Host.AidaClient.RpcGlobalId, rate: Host.AidaClient.Rating, feedback?: string) => void,
12161207
onLastAnswerMarkdownViewRef: (el: Element|undefined) => void,
12171208
onMessageContainerRef: (el: Element|undefined) => void,
1218-
render: () => void,
12191209
}): Lit.TemplateResult {
12201210
// clang-format off
12211211
return html`
@@ -1232,7 +1222,6 @@ function renderMessages({
12321222
onSuggestionClick,
12331223
onFeedbackSubmit,
12341224
onLastAnswerMarkdownViewRef,
1235-
render
12361225
}),
12371226
)}
12381227
</div>
@@ -1575,7 +1564,6 @@ function renderMainContents({
15751564
onFeedbackSubmit,
15761565
onLastAnswerMarkdownViewRef,
15771566
onMessageContainerRef,
1578-
render,
15791567
}: {
15801568
state: State,
15811569
aidaAvailability: Host.AidaClient.AidaAccessPreconditions,
@@ -1598,7 +1586,6 @@ function renderMainContents({
15981586
void,
15991587
onLastAnswerMarkdownViewRef: (el: Element|undefined) => void,
16001588
onMessageContainerRef: (el: Element|undefined) => void,
1601-
render: () => void,
16021589
}): Lit.TemplateResult {
16031590
if (state === State.CONSENT_VIEW) {
16041591
return renderDisabledState(consentViewContents);
@@ -1624,7 +1611,6 @@ function renderMainContents({
16241611
onFeedbackSubmit,
16251612
onLastAnswerMarkdownViewRef,
16261613
onMessageContainerRef,
1627-
render
16281614
});
16291615
}
16301616

0 commit comments

Comments
 (0)