From aa80dbdd3e58e79401008665f2199d98fa14b3d0 Mon Sep 17 00:00:00 2001
From: jotaen4tinypilot <83721279+jotaen4tinypilot@users.noreply.github.com>
Date: Wed, 23 Aug 2023 17:36:35 +0200
Subject: [PATCH] Make for smooth vertical resizing of remote screen (#1583)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Part of https://github.com/tiny-pilot/tinypilot/issues/728.

The vertical resizing behaviour of the remote screen is currently so
that the screen element’s height appears to “jump” at certain steps due
to [CSS
breakpoints](https://github.com/tiny-pilot/tinypilot/blob/069cf2d9829a4fe2b8df33cb03b26c141c60593b/app/templates/custom-elements/remote-screen.html#L11-L33),
while you resize the window vertically.

This PR implements “smooth” and step-less vertical resizing, while
maintaining a constant margin between remote screen and status-bar
during that process.

The breakpoint-less resizing is important for the “dedicated window”
(popup) that we are about to implement in
https://github.com/tiny-pilot/tinypilot/issues/728, where it would
probably feel weird if the remote screen height jumped around the way it
currently does, or if we wouldn’t be able to fill up the entire vertical
space of the popup window due to the breakpoint setup.

## Demo: After


https://github.com/tiny-pilot/tinypilot/assets/83721279/66758c7c-ce78-4f0a-8323-4990be47fe49

## Demo: Before


https://github.com/tiny-pilot/tinypilot/assets/83721279/f768b3c0-8b5b-44c3-8cbd-2ee724e46a02

## Notes

- We had introduced these breakpoints in
https://github.com/tiny-pilot/tinypilot/pull/878 originally, as kind of
“heuristic”/pragmatic approach for resizing the remote screen, without
having the remote screen visually collide with the status bar. The
breakpoints itself are arbitrary, and they don’t serve any other purpose
except for facilitating the vertical screen size while resizing the
window.
- I discovered https://github.com/tiny-pilot/tinypilot/issues/1581 while
testing this PR; so this behaviour is not a regression, but it’s
currently broken on master.
- This PR leaves a “naked” `<div>` in `index.html` which we technically
don’t need anymore. I’ll clean that up in a later PR, to avoid a noisy
whitespace diff here.
- The `page-content` CSS class is orphaned, so I’ve removed it.

I’ve tested on device with Firefox and Chrome, and I tried to check all
imaginable scaling constellations of window sizes and aspect ratios,
both with MJPEG and H.264, and both in full screen and regular window
mode. The biggest risk of this PR would be that we accidentally mess
with the mouse cursor positioning/alignment. It would be cool if you
could double-check on device as well, with a real target screen, just so
that we are safe.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1583"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
---
 app/static/css/style.css                      |  9 ----
 .../custom-elements/remote-screen.html        | 44 +++++++++----------
 app/templates/index.html                      |  2 +-
 3 files changed, 21 insertions(+), 34 deletions(-)

diff --git a/app/static/css/style.css b/app/static/css/style.css
index 3013ff189..c3100553b 100644
--- a/app/static/css/style.css
+++ b/app/static/css/style.css
@@ -61,15 +61,6 @@ body {
   }
 }
 
-.container {
-  display: flex;
-  flex-direction: column;
-  justify-content: center;
-  text-align: center;
-  margin: 0 auto;
-  padding-top: 55px; /* Account for menu bar plus some extra spacing */
-}
-
 .header-bar {
   position: fixed;
   top: 0;
diff --git a/app/templates/custom-elements/remote-screen.html b/app/templates/custom-elements/remote-screen.html
index 8d5f925f2..896318b01 100644
--- a/app/templates/custom-elements/remote-screen.html
+++ b/app/templates/custom-elements/remote-screen.html
@@ -2,34 +2,30 @@
   <style>
     @import "css/cursors.css";
 
-    .screen {
-      max-width: 100%;
-      max-height: 70vh;
-      object-fit: contain;
-    }
-
-    @media screen and (min-height: 450px) {
-      .screen {
-        max-height: 80vh;
-      }
+    :host {
+      --menu-bar-height: 45px;
+      --status-bar-height: 31px;
+      display: flex;
+      flex-direction: column;
+      box-sizing: border-box;
+      height: 100vh;
+      padding-top: calc(var(--menu-bar-height) + 10px);
+      padding-bottom: calc(var(--status-bar-height) + 10px);
+      align-items: center;
     }
 
-    @media screen and (min-height: 600px) {
-      .screen {
-        max-height: 85vh;
-      }
-    }
-
-    @media screen and (min-height: 900px) {
-      .screen {
-        max-height: 90vh;
-      }
+    .screen-wrapper {
+      display: flex;
+      flex-direction: column;
+      align-items: center;
+      max-width: 100%;
+      max-height: 100%;
     }
 
-    @media screen and (min-height: 1180px) {
-      .screen {
-        max-height: 100vh;
-      }
+    .screen {
+      max-width: 100%;
+      max-height: 100%;
+      object-fit: contain;
     }
 
     :host([fullscreen="true"]) .screen-wrapper {
diff --git a/app/templates/index.html b/app/templates/index.html
index ba2dc5216..ab4df8c83 100644
--- a/app/templates/index.html
+++ b/app/templates/index.html
@@ -25,7 +25,7 @@
         <status-bar id="status-bar"></status-bar>
       </div>
 
-      <div class="page-content container">
+      <div>
         <overlay-panel id="error-overlay" variant="danger">
           <error-dialog id="error-dialog"></error-dialog>
         </overlay-panel>