Mobile Bridge #1746

chrismgeorge · 2020-12-04T17:33:12Z

Design doc: (go/modelviewer-mobile-bridge)

Looking for a code review and a feature review. Especially interested in the usability of Scene Viewer.

chrismgeorge · 2020-12-04T17:34:00Z

tagging @mounirlamouri for visibility

chrismgeorge · 2020-12-10T21:01:20Z

design doc: (go/modelviewer-mobile-bridge)

…the editor knows when mobile is ready

…ith-qr-code

chrismgeorge · 2020-12-15T19:54:53Z

packages/space-opera/src/components/mobile_view/components/mobile_modal.ts

+import {ConnectedLitElement} from '../../connected_lit_element/connected_lit_element.js';
+
+@customElement('mobile-modal')
+export class MobileModal extends ConnectedLitElement {


Modal for displaying the QR Code & link

chrismgeorge · 2020-12-15T19:55:35Z

packages/space-opera/src/components/mobile_view/mobile_view.ts

+ * The view loaded at /editor/view/?id=xyz
+ */
+@customElement('mobile-view')
+export class MobileView extends ConnectedLitElement {


What gets rendered when we are looking at editor/view/id?=1234 on mobile

chrismgeorge · 2020-12-15T19:57:55Z

packages/space-opera/src/components/mobile_view/mobile_view.ts

+  async fetchLoop() {
+    await fetch(this.updatesPipeUrl)
+        .then(response => response.json())
+        .then(json => this.waitForData(json))


json is the initial object sent that has properties that determine what has changed: i.e.:
{gltfChanged: boolean, stateChanged: boolean, envChanged: boolean, envIsHdr: boolean}
and is how we dynamically know what to GET

Generally you either use .then or await but not both, since they do the same thing. Let's stick with await.

chrismgeorge · 2020-12-15T20:00:05Z

packages/space-opera/src/components/mobile_view/open_mobile_view.ts

+ * Section for displaying QR Code and other info related to mobile
+ */
+@customElement('open-mobile-view')
+export class OpenMobileView extends ConnectedLitElement {


New section underneath the file manager for displaying the ar & mobile features

chrismgeorge · 2020-12-15T20:02:22Z

packages/space-opera/src/components/mobile_view/open_mobile_view.ts

+    `
+  }
+
+  render() {


after deployed: https://screenshot.googleplex.com/5G9AFik727gbzyS.png

not refreshed: https://screenshot.googleplex.com/BaUSavVQFqQuEwh.png

sending data: https://screenshot.googleplex.com/7tSHxWr7FkN6oiD.png

chrismgeorge · 2020-12-15T20:04:53Z

packages/space-opera/src/components/mobile_view/components/mobile_modal.ts

+    this.isOpen = false;
+  }
+
+  render() {


modal: https://screenshot.googleplex.com/3qyLTW4HNQLww2p.png

chrismgeorge · 2020-12-15T20:05:56Z

packages/space-opera/src/components/mobile_view/reducer.ts

+export const getArConfig = (state: State) =>
+    state.entities.modelViewerSnippet.arConfig;
+
+export function arReducer(


As more things are added, this reducer may become the "mobileReducer" to house all of the snippet config without a home and relevant to mobile, like iosSrc

chrismgeorge · 2020-12-15T20:07:58Z

packages/space-opera/src/components/model_viewer_snippet/model_viewer_snippet.ts

  @query('me-import-card') importCard!: ImportCard;

+  isTestingMobile(): boolean {
+    return window.location.search === '?id=testingMobile';


The mobile view section is only visible at

/editor/?id=testingMobile

chrismgeorge · 2020-12-15T23:29:17Z

Added a Logical Flow section to the design doc: https://docs.google.com/document/d/1JHWioVxrhfwZyPMccRUyYqP4cl2Z0YggcpXl4UFyTmo/edit#heading=h.uh3hs9gwqhmu

Detailing the GET/POSTs that are being sent and listened for.

Importantly:

elalish

I'd like these comments addressed, but I think it's good enough to merge as-is, especially since it's behind a flag. I'd like to get testing it in it's deployed state sooner than later.

elalish · 2020-12-16T01:13:47Z

packages/space-opera/editor/view/index.html

+  <!-- Web animations for paper-dropdown -->
+  <script src="../../node_modules/web-animations-js/web-animations-next-lite.min.js"></script>
+  <script src="../../node_modules/js-beautify/js/lib/beautify-html.js"></script>
+  <script src="../../node_modules/js-beautify/js/lib/beautify-css.js"></script>


Are these dependencies needed when there's no UI?

elalish · 2020-12-16T01:19:08Z

packages/space-opera/src/components/mobile_view/mobile_view.ts

+        .then(blob => {
+          // simulating createBlobUrlFromEnvironmentImage
+          const addOn = envIsHdr ? '#.hdr' : '';
+          const envUrl = URL.createObjectURL(blob) + addOn;


Looks like you're working around one of model-viewer's hacks here. We should really fix up how we recognize HDRs someday...

elalish · 2020-12-16T01:20:34Z

packages/space-opera/src/components/mobile_view/mobile_view.ts

+      await this.waitForState(json.envChanged);
+    }
+    if (json.envChanged) {
+      await this.waitForEnv(json.envIsHdr);


Do these really happen one after the other, or should they be in parallel?

One after another. The important part is when the state snippet needs to be dispatch its data. It overrides information of the gltf and environment url.

elalish · 2020-12-16T01:21:25Z

packages/space-opera/src/components/mobile_view/mobile_view.ts

+  async fetchLoop() {
+    await fetch(this.updatesPipeUrl)
+        .then(response => response.json())
+        .then(json => this.waitForData(json))


Generally you either use .then or await but not both, since they do the same thing. Let's stick with await.

elalish · 2020-12-16T01:22:00Z

packages/space-opera/src/components/mobile_view/mobile_view.ts

+    await fetch(this.updatesPipeUrl)
+        .then(response => response.json())
+        .then(json => this.waitForData(json))
+        .catch((error) => {


Can you use .catch outside of a try block?

elalish · 2020-12-16T01:32:45Z

packages/space-opera/src/components/mobile_view/open_mobile_view.ts

+    const dropdown = event.target as Dropdown;
+    const key = dropdown.selectedItem?.getAttribute('value') || undefined;
+    if (key === 'default' || key === 'webxr') {
+      reduxStore.dispatch(dispatchArModes('webxr scene-viewer quick-look'));


Let's simplify this. Maybe just a checkbox to default to scene-viewer instead of webxr? Quicklook can be always enabled, since it's the only option on iphones, and it's really controlled by the presence of an ios-src.

chrismgeorge added 4 commits December 2, 2020 07:01

Merge branch 'master' of github.com:google/model-viewer

bee63a5

Merge branch 'master' of github.com:google/model-viewer

ab24f6d

Merge branch 'master' of github.com:google/model-viewer

4b75adc

initial logic for UI for mobile view

069456c

chrismgeorge added 7 commits December 4, 2020 12:32

add posts

7a3f0e7

add qr code capability via npm package qrious

07b7cec

name changes

858b6a1

add basic routing

6011f71

move mobile to own expandable section

d81698a

add todos

862cc61

env image, glb, poster, and snippet receive and updates

b1b529f

chrismgeorge changed the title ~~Mobile with qr code~~ Mobile Bridge Dec 10, 2020

chrismgeorge added 2 commits December 10, 2020 09:20

remove router

2160ee0

add catch statements and use an update object to initiate mobile view

cb05fd1

chrismgeorge added 14 commits December 10, 2020 14:24

add modal for mobile view instead of in bar

0208b56

searching for the listener...

60530f2

fetching loops such that updates are constantly being asked for, and …

fb67acd

…the editor knows when mobile is ready

Merge branch 'master' of github.com:google/model-viewer into mobile-w…

1c0e5e1

…ith-qr-code

testing tests

0f3644d

remove poster, fix async everything but applying material edits

11e5c53

add random number for session

15e5593

add ar toggle

b31fab5

initial test of ar modes

48db602

add ar modes, dynamic refresh button if viewers out of sync

83fc24e

get materials to update by sending packed model

a1f20da

code cleanup

c46606c

moved open section into mobile view

00803a3

fix wording of modal

a4eb8f9

chrismgeorge commented Dec 15, 2020

View reviewed changes

cleaning up comments

784a42e

chrismgeorge requested a review from elalish December 15, 2020 20:12

elalish approved these changes Dec 16, 2020

View reviewed changes

chrismgeorge merged commit aa2587c into master Dec 16, 2020

chrismgeorge deleted the mobile-with-qr-code branch December 16, 2020 18:30

nwtgck mentioned this pull request Jul 1, 2025

Glitch hosting termination affecting Piping Server instance #5061

Closed

samaneh-kazemi mentioned this pull request Jul 11, 2025

update Piping server instance #5070

Merged

Mobile Bridge #1746

Mobile Bridge #1746

Uh oh!

Conversation

chrismgeorge commented Dec 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrismgeorge commented Dec 4, 2020

Uh oh!

chrismgeorge commented Dec 10, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrismgeorge Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrismgeorge Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrismgeorge Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrismgeorge commented Dec 15, 2020

Uh oh!

elalish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chrismgeorge commented Dec 4, 2020 •

edited

Loading

chrismgeorge Dec 15, 2020 •

edited

Loading

chrismgeorge Dec 15, 2020 •

edited

Loading

chrismgeorge Dec 15, 2020 •

edited

Loading