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

✨ Assist.js: Add a shared FrameService to assistjs extension #32784

Merged
merged 35 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
21ffb96
Create the skeleton of the new extension for assist.js: amp-google-as…
ychsieh Jan 26, 2021
c44c038
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Jan 26, 2021
8a194be
Remove unneeded regenerator-
ychsieh Jan 26, 2021
d0844da
- Add/modify copyright statement and header comment for each file
ychsieh Jan 27, 2021
b18e9c2
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Jan 27, 2021
66e40b7
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 2, 2021
3662c29
Add a shared service(AssistjsConfigService) and a new config element.…
ychsieh Feb 2, 2021
dae219b
Fix styling and remove commented code in AssistjsFrameService.
ychsieh Feb 2, 2021
b7a97d4
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 5, 2021
c4eab76
Improve config service:
ychsieh Feb 5, 2021
a7be424
Fix prettify errors.
ychsieh Feb 5, 2021
2654cd6
Unit test fix and some minor fixes.
ychsieh Feb 8, 2021
3ab4470
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 8, 2021
c0a830f
Fix lint errors.
ychsieh Feb 8, 2021
d317cb3
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 9, 2021
09d7d5b
Update extensions/amp-google-assistant-assistjs/0.1/assistjs-frame-se…
ychsieh Feb 12, 2021
bda9bb7
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 12, 2021
3c2e7df
Merge branch 'assistjs' of https://github.com/ychsieh/amphtml into as…
ychsieh Feb 12, 2021
615589e
Minor fixes.
ychsieh Feb 12, 2021
2070b0b
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 16, 2021
88e193e
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 17, 2021
8464e66
Fix errors caused by not having this.win in a service class(non-AMP.B…
ychsieh Feb 17, 2021
6b471fd
ampdoc.getWin() is forbidden. Replace it with "window" directly.
ychsieh Feb 17, 2021
88a525d
- Append iframe to the document after frame src is retrieved
ychsieh Feb 19, 2021
88cb168
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 19, 2021
0bcce79
Fix FrameService reference errors and lint errors.
ychsieh Feb 19, 2021
c165319
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 22, 2021
98d516e
Add a shared FrameService for all custom elements to handle requests …
ychsieh Feb 22, 2021
67d62c0
Use ampdoc.whenready instead to fix the loading error.
ychsieh Feb 22, 2021
63e2c2a
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 23, 2021
f4c4dc9
Update extensions/amp-google-assistant-assistjs/0.1/assistjs-frame-se…
ychsieh Feb 24, 2021
88f9ce9
Update extensions/amp-google-assistant-assistjs/0.1/assistjs-frame-se…
ychsieh Feb 24, 2021
266a660
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 24, 2021
7a380ca
Merge branch 'assistjs' of https://github.com/ychsieh/amphtml into as…
ychsieh Feb 24, 2021
4135608
Merge remote-tracking branch 'upstream/master' into assistjs
ychsieh Feb 25, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {AmpGoogleAssistantInlineSuggestionBar} from './amp-google-assistant-inli
import {AmpGoogleAssistantVoiceBar} from './amp-google-assistant-voice-bar';
import {AmpGoogleAssistantVoiceButton} from './amp-google-assistant-voice-button';
import {AssistjsConfigService} from './assistjs-config-service';
import {AssistjsFrameService} from './assistjs-frame-service';
import {CSS} from '../../../build/amp-google-assistant-assistjs-0.1.css';
import {Services} from '../../../src/services';

Expand All @@ -38,6 +39,7 @@ export class AmpGoogleAssistantAssistjsConfig extends AMP.BaseElement {

AMP.extension('amp-google-assistant-assistjs', '0.1', (AMP) => {
AMP.registerServiceForDoc('assistjs-config-service', AssistjsConfigService);
AMP.registerServiceForDoc('assistjs-frame-service', AssistjsFrameService);
AMP.registerElement(
'amp-google-assistant-assistjs-config',
AmpGoogleAssistantAssistjsConfig,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,15 @@ export class AmpGoogleAssistantInlineSuggestionBar extends AMP.BaseElement {

/** @private {?AssistjsConfigService} */
this.configService_ = null;

/** @private {?AssistjsFrameService} */
this.frameService_ = null;
}

/** @override */
buildCallback() {
this.configService_ = Services.assistjsConfigServiceForDoc(this.element);
this.frameService_ = Services.assistjsFrameServiceForDoc(this.element);
}

/** @override */
Expand All @@ -51,6 +55,7 @@ export class AmpGoogleAssistantInlineSuggestionBar extends AMP.BaseElement {
.then((iframeUrl) => {
addAttributesToElement(iframe, {
src: iframeUrl,
sandbox: 'allow-scripts',
});

// applyFillContent so that frame covers the entire component.
Expand All @@ -59,6 +64,11 @@ export class AmpGoogleAssistantInlineSuggestionBar extends AMP.BaseElement {
this.element.appendChild(iframe);
});

iframe.addEventListener('load', () => {
// TODO: create a channel to receive requests from underlying assist.js iframe.
this.frameService_.sendTextQuery();
});

// Return a load promise for the frame so the runtime knows when the
// component is ready.
return this.loadPromise(iframe);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,15 @@ export class AmpGoogleAssistantVoiceBar extends AMP.BaseElement {

/** @private {?AssistjsConfigService} */
this.configService_ = null;

/** @private {?AssistjsFrameService} */
this.frameService_ = null;
}

/** @override */
buildCallback() {
this.configService_ = Services.assistjsConfigServiceForDoc(this.element);
this.frameService_ = Services.assistjsFrameServiceForDoc(this.element);
}

/** @override */
Expand All @@ -49,6 +53,7 @@ export class AmpGoogleAssistantVoiceBar extends AMP.BaseElement {
this.configService_.getWidgetIframeUrl('voicebar').then((iframeUrl) => {
addAttributesToElement(iframe, {
src: iframeUrl,
sandbox: 'allow-scripts',
});

// applyFillContent so that frame covers the entire component.
Expand All @@ -57,6 +62,12 @@ export class AmpGoogleAssistantVoiceBar extends AMP.BaseElement {
this.element.appendChild(iframe);
});

iframe.addEventListener('load', () => {
// TODO: create a channel to receive requests from underlying assist.js iframe.
this.frameService_.openMic();
this.frameService_.sendTextQuery();
});

// Return a load promise for the frame so the runtime knows when the
// component is ready.
return this.loadPromise(iframe);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,15 @@ export class AmpGoogleAssistantVoiceButton extends AMP.BaseElement {

/** @private {?AssistjsConfigService} */
this.configService_ = null;

/** @private {?AssistjsFrameService} */
this.frameService_ = null;
}

/** @override */
buildCallback() {
this.configService_ = Services.assistjsConfigServiceForDoc(this.element);
this.frameService_ = Services.assistjsFrameServiceForDoc(this.element);
}

/** @override */
Expand All @@ -49,6 +53,7 @@ export class AmpGoogleAssistantVoiceButton extends AMP.BaseElement {
this.configService_.getWidgetIframeUrl('voicebutton').then((iframeUrl) => {
addAttributesToElement(iframe, {
src: iframeUrl,
sandbox: 'allow-scripts',
});

// applyFillContent so that frame covers the entire component.
Expand All @@ -57,6 +62,11 @@ export class AmpGoogleAssistantVoiceButton extends AMP.BaseElement {
this.element.appendChild(iframe);
});

iframe.addEventListener('load', () => {
// TODO: create a channel to receive requests from underlying assist.js iframe.
this.frameService_.openMic();
});

// Return a load promise for the frame so the runtime knows when the
// component is ready.
return this.loadPromise(iframe);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/**
* Copyright 2021 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* @fileoverview The shared service used by all custom elements to talk to Assistant services. It loads an iframe that exports
* endpoints to handle requests from custom elements.
*/

import {Services} from '../../../src/services';
import {addAttributesToElement} from '../../../src/dom';
import {toggle} from '../../../src/style';

export class AssistjsFrameService {
/**
* @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
*/
constructor(ampdoc) {
/** @private @const {!../../../src/service/ampdoc-impl.AmpDoc} */
this.ampDoc_ = ampdoc;

/** @private {?AssistjsConfigService} */
this.configService_ = null;

/** Create Assistant iframe and append it to the main AMP document. */
this.createAssistantIframe_();
}

/** @private */
createAssistantIframe_() {
this.ampDoc_.whenReady().then(() => {
ychsieh marked this conversation as resolved.
Show resolved Hide resolved
this.configService_ = Services.assistjsConfigServiceForDoc(this.ampDoc_);
const iframe = window.document.createElement('iframe');
ychsieh marked this conversation as resolved.
Show resolved Hide resolved
this.configService_.getWidgetIframeUrl('frame').then((iframeUrl) => {
addAttributesToElement(iframe, {
src: iframeUrl,
allow: 'microphone',
sandbox: 'allow-scripts',
});
toggle(iframe, false);
document.body.appendChild(iframe);
});
});
}

/**
* Activates Assistant microphone on 3P page.
*/
openMic() {
// TODO: add implementation once the channels for iframes are implemented.
}

/**
* Sends text query to Assistant server.
*/
sendTextQuery() {
// TODO: add implementation once the channels for iframes are implemented.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ describes.realWin(
.withExactArgs('voicebutton')
.once();

const frameServiceMock = env.sandbox.mock(
Services.assistjsFrameServiceForDoc(document)
);
frameServiceMock.expects('openMic').once();

configElement = document.createElement(
'amp-google-assistant-assistjs-config'
);
Expand Down