Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(app): fix run preview when full-screened #16000

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Aug 14, 2024

Closes RQA-3005

Overview

We were hardcoding widths for the default app sizing but not considering custom sizing. Although we don't really support non-default dimensions in a lot of ways, it's something that would be probably be a good idea to refactor towards. Ideally, we could do something like height:100% here and have it work out, but we need to position:fixed the protocol run header, add more containers, handle the overflows correctly, and insure every child component still looks correct, which produces a much larger bug surface. The viewport approach is a bit more direct.

Current Behavior

Screen.Recording.2024-08-14.at.2.30.33.PM.mov

Fixed Behavior

Screen.Recording.2024-08-14.at.2.29.57.PM.mov

Test Plan and Hands on Testing

  • See videos

Risk assessment

low

@mjhuff mjhuff requested review from SyntaxColoring, TamarZanzouri and a team August 14, 2024 18:38
@mjhuff mjhuff requested a review from a team as a code owner August 14, 2024 18:38
@mjhuff mjhuff requested review from jbleon95, Elyorcv, koji, ncdiehl11 and jerader and removed request for a team, jbleon95 and Elyorcv August 14, 2024 18:38
Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this sounds like it makes sense if we can't get all the way to height: 100% now. TY!

Copy link
Contributor

@TamarZanzouri TamarZanzouri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@koji koji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mjhuff mjhuff merged commit 708bea3 into chore_release-8.0.0 Aug 14, 2024
26 checks passed
@mjhuff mjhuff deleted the app_run-preview-fullscreen-css branch August 14, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants