Re: [w3c/screen-orientation] Editorial: Example 1 (#133)

marcoscaceres commented on this pull request.

Rapid fire suggestions :) 

> @@ -529,28 +529,50 @@ <h2>
         Examples
       </h2>
       <p>
-        This example shows the current screen orientation to the console every
-        time the screen orientation state changes.
+        In this example for a mobile phone, clicking the landscape button makes

```suggestion
        In this example for a mobile device, clicking the landscape button makes
```

> @@ -529,28 +529,50 @@ <h2>
         Examples
       </h2>
       <p>
-        This example shows the current screen orientation to the console every
-        time the screen orientation state changes.
+        In this example for a mobile phone, clicking the landscape button makes
+        a request to go fullscreen and then lock to landscape orientation. Once
+        it is in landscape, the unlock button will take the screen orientation

```suggestion
        it is in landscape, pressing the unlock button reverts the screen orientation
```

> @@ -529,28 +529,50 @@ <h2>
         Examples
       </h2>
       <p>
-        This example shows the current screen orientation to the console every
-        time the screen orientation state changes.
+        In this example for a mobile phone, clicking the landscape button makes
+        a request to go fullscreen and then lock to landscape orientation. Once
+        it is in landscape, the unlock button will take the screen orientation
+        back to default (portrait) or the portrait button will also change it

```suggestion
        back to default (portrait). Pressing the button labelled "portrait" will also change it
```

> @@ -529,28 +529,50 @@ <h2>
         Examples
       </h2>
       <p>
-        This example shows the current screen orientation to the console every
-        time the screen orientation state changes.
+        In this example for a mobile phone, clicking the landscape button makes
+        a request to go fullscreen and then lock to landscape orientation. Once
+        it is in landscape, the unlock button will take the screen orientation
+        back to default (portrait) or the portrait button will also change it
+        back to portrait.

```suggestion
        back to <a>portrait</a>.
```

> @@ -529,28 +529,50 @@ <h2>
         Examples
       </h2>
       <p>
-        This example shows the current screen orientation to the console every
-        time the screen orientation state changes.
+        In this example for a mobile phone, clicking the landscape button makes
+        a request to go fullscreen and then lock to landscape orientation. Once
+        it is in landscape, the unlock button will take the screen orientation
+        back to default (portrait) or the portrait button will also change it
+        back to portrait.
+      </p>
+      <p>
+        The log shows the change in orientation type and angle, and whether the
+        event type is load or change.

```suggestion
        event type is "load" or "change".
```

> @@ -529,28 +529,50 @@ <h2>
         Examples
       </h2>
       <p>
-        This example shows the current screen orientation to the console every
-        time the screen orientation state changes.
+        In this example for a mobile phone, clicking the landscape button makes
+        a request to go fullscreen and then lock to landscape orientation. Once
+        it is in landscape, the unlock button will take the screen orientation
+        back to default (portrait) or the portrait button will also change it
+        back to portrait.
+      </p>
+      <p>
+        The log shows the change in orientation type and angle, and whether the
+        event type is load or change.

Or perhaps, ", and the type of event."

>        </p>
       <pre class='example html'>
 &lt;script&gt;
-  var show = function() {
-     console.log("Orientation type is " + screen.orientation.type);
-     console.log("Orientation angle is " + screen.orientation.angle);
-  }
+async function lockLandscape(){
+  await document.documentElement.requestFullscreen() 

```suggestion
  await document.documentElement.requestFullscreen();
```

>        </p>
       <pre class='example html'>
 &lt;script&gt;
-  var show = function() {
-     console.log("Orientation type is " + screen.orientation.type);
-     console.log("Orientation angle is " + screen.orientation.angle);
-  }
+async function lockLandscape(){
+  await document.documentElement.requestFullscreen() 

I think we might actually want to do the `checkIfFullScreen()` function, otherwise, it looks like you always have to call requestFullScreen() before using the API. 

For `checkIfFullScreen()`, you will need to figure out how to check if fullscreen is enabled (MDN should probably be able to tell you). Basically, if fullscreen is already enabled, then just return, otherwise, call requestFullscreen(); and return the resulting promise. 

>        </p>
       <pre class='example html'>
 &lt;script&gt;
-  var show = function() {
-     console.log("Orientation type is " + screen.orientation.type);
-     console.log("Orientation angle is " + screen.orientation.angle);
-  }
+async function lockLandscape(){
+  await document.documentElement.requestFullscreen() 
+  await screen.orientation.lock('landscape');
+}
+
+async function unlock() {
+  await screen.orientation.unlock();
+}
+
+async function portrait(){
+  await screen.orientation.lock('portrait');

Nit: here you need the full screen check... otherwise, this will fail if the button is just pressed. 

>  
-  screen.orientation.addEventListener("change", show);
-  window.onload = show;
+const { type, angle } = screen.orientation
+function show(event) {
+ console.log("Orientation type is " + screen.orientation.type);
+ console.log("Orientation angle is " + screen.orientation.angle);
+ console.log("Event Type is " + event.type)
+}
+
+screen.orientation.addEventListener("change", show);
+window.onload = show;

Just to be consistent with previous line: 

```suggestion
window.addEventListener("load", show);
```

>  &lt;/button&gt;
-&lt;button onclick="screen.orientation.lock('landscape')"&gt;
-  Lock to landscape
+&lt;button onclick="portrait()"&gt;
+  Portrait

What might be cute is to "rotate" instead... you could check the orientation, then switch from one to the other. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/screen-orientation/pull/133#pullrequestreview-192659805

Received on Tuesday, 15 January 2019 14:14:02 UTC