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

report: remove extra space from screenshot preview #14424

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

adamraine
Copy link
Member

Before:

Screen Shot 2022-10-04 at 2 43 03 PM

After:

Screen Shot 2022-10-04 at 2 43 18 PM

@adamraine adamraine requested a review from a team as a code owner October 4, 2022 21:44
@adamraine adamraine requested review from connorjclark and removed request for a team October 4, 2022 21:44
@adamraine adamraine merged commit 944a2cf into main Oct 4, 2022
@adamraine adamraine deleted the rm-screenshot-gutter branch October 4, 2022 22:51
@paulirish
Copy link
Member

@adamraine Just ran into a regression which I think is related to this change...

image

The preview thumbnail is 5300px talll

Repro is theverge. mobile. offscreen-images audit. But... I was scrolling during gathering, sooooo...

without much thinking i attempted.....

    if (elementPreviewSizeSC.width < elementPreviewSizeSC.height) {
      elementPreviewSizeSC.width = Math.min(screenshot.width, elementPreviewSizeSC.width);
    } else {
      elementPreviewSizeSC.height = Math.min(screenshot.height, elementPreviewSizeSC.height);
    }

and the results looked good. but I'm not sure about the impact on the example.com testcase that motivated this.
Can you take a look?

@adamraine
Copy link
Member Author

I was scrolling during gathering, sooooo...

I haven't gotten this audit to fail unless I'm scrolling. Have you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants