Skip to content

Commit

Permalink
Make for smooth vertical resizing of remote screen (#1583)
Browse files Browse the repository at this point in the history
Part of #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
#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
#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 #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>
  • Loading branch information
jotaen4tinypilot authored Aug 23, 2023
1 parent a6a36fc commit aa80dbd
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 34 deletions.
9 changes: 0 additions & 9 deletions app/static/css/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
44 changes: 20 additions & 24 deletions app/templates/custom-elements/remote-screen.html
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion app/templates/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit aa80dbd

Please sign in to comment.