Skip to content

Commit

Permalink
- Hotfix: address fullscreen fallback in Chrome m76
Browse files Browse the repository at this point in the history
 - Document the nature of the workaround
 - Add note about order of click handlers
 - v0.5.1
  • Loading branch information
Chris Joel authored and cdata committed Aug 13, 2019
1 parent 6eb788f commit 5381bdf
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 4 deletions.
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@google/model-viewer",
"version": "0.5.0",
"version": "0.5.1",
"description": "Easily display interactive 3D models on the web and in AR!",
"repository": "https://github.com/GoogleWebComponents/model-viewer",
"bugs": {
Expand Down
30 changes: 29 additions & 1 deletion src/features/ar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ const $arMode = Symbol('arMode');
const $canLaunchQuickLook = Symbol('canLaunchQuickLook');
const $quickLookBrowsers = Symbol('quickLookBrowsers');

const $arButtonContainerFallbackClickHandler =
Symbol('arButtonContainerFallbackClickHandler');
const $onARButtonContainerFallbackClick =
Symbol('onARButtonContainerFallbackClick');
const $arButtonContainerClickHandler = Symbol('arButtonContainerClickHandler');
const $onARButtonContainerClick = Symbol('onARButtonContainerClick');

Expand Down Expand Up @@ -157,6 +161,16 @@ export const ARMixin = (ModelViewerElement:
this.shadowRoot!.querySelector('#default-exit-fullscreen-button') as
HTMLElement;

// NOTE(cdata): We use a second, separate "fallback" click handler in
// order to work around a regression in how Chrome on Android behaves
// when requesting fullscreen at the same time as triggering an intent.
// As of m76, intents could no longer be triggered successfully if they
// were dispatched in the same handler as the fullscreen request. The
// workaround is to split both effects into their own event handlers.
// @see https://github.com/GoogleWebComponents/model-viewer/issues/693
protected[$arButtonContainerFallbackClickHandler] = (event: Event) =>
this[$onARButtonContainerFallbackClick](event);

protected[$arButtonContainerClickHandler]: (event: Event) => void =
(event) => this[$onARButtonContainerClick](event);

Expand Down Expand Up @@ -185,7 +199,6 @@ export const ARMixin = (ModelViewerElement:
await this[$enterARWithWebXR]();
break;
case ARMode.AR_VIEWER:
this.requestFullscreen();
openARViewer(this.src!, this.alt || '');
break;
default:
Expand Down Expand Up @@ -300,19 +313,34 @@ configuration or device capabilities');

if (showArButton) {
this[$arButtonContainer].classList.add('enabled');
// NOTE(cdata): The order of the two click handlers on the "ar
// button container" is important, vital to the workaround described
// earlier in this file. Reversing their order will cause our Scene
// Viewer integration to break.
// @see https://github.com/GoogleWebComponents/model-viewer/issues/693
this[$arButtonContainer].addEventListener(
'click', this[$arButtonContainerClickHandler]);
this[$arButtonContainer].addEventListener(
'click', this[$arButtonContainerFallbackClickHandler]);
this[$exitFullscreenButtonContainer].addEventListener(
'click', this[$exitFullscreenButtonContainerClickHandler]);
} else {
this[$arButtonContainer].removeEventListener(
'click', this[$arButtonContainerClickHandler]);
this[$arButtonContainer].removeEventListener(
'click', this[$arButtonContainerFallbackClickHandler]);
this[$exitFullscreenButtonContainer].removeEventListener(
'click', this[$exitFullscreenButtonContainerClickHandler]);
this[$arButtonContainer].classList.remove('enabled');
}
}

[$onARButtonContainerFallbackClick](_event: Event) {
if (this[$arMode] === ARMode.AR_VIEWER) {
this.requestFullscreen();
}
}

[$onARButtonContainerClick](event: Event) {
event.preventDefault();
this.activateAR();
Expand Down
2 changes: 1 addition & 1 deletion src/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ template.innerHTML = `
<div class="slot ar-button">
<slot name="ar-button">
<a id="default-ar-button" class="fab" href="#"
<a id="default-ar-button" class="fab"
tabindex="2"
aria-label="View this 3D model up close">
${ARGlyph}
Expand Down

0 comments on commit 5381bdf

Please sign in to comment.