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

Update site screenshots to hide scrollbars on Windows #133

Merged
merged 1 commit into from
May 15, 2024

Conversation

derekblank
Copy link
Contributor

Resolves Automattic/dotcom-forge#7077

Proposed Changes

Updates CSS specificity in createScreenshotWindow waitForCapture to further hide scrollbars on Windows

Testing Instructions

  1. Create site, and change theme to Seedlet
  2. Start site on Windows
  3. Observe that screenshot does not contain scrollbars (on macOS, behavior should be unchanged)

Note

This issue is difficult to replicate consistently. While this change should improve the behavior of not displaying scrollbars in screenshots on Windows, there may be a separate issue where either the theme still overrides the injected CSS, or the screenshot is captured before the extra CSS is injected.

Before After
before after

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@derekblank derekblank added [Type] Bug Something isn't working [Platform] Windows The issue is specific to the Windows platform. labels May 15, 2024
@derekblank derekblank requested a review from a team May 15, 2024 08:42
@derekblank
Copy link
Contributor Author

derekblank commented May 15, 2024

@wojtekn As referenced in the PR notes, I found it difficult to replicate the display of the scrollbars consistently. In all cases on macOS, the scrollbars were hidden. In some cases on Windows, repeatedly starting and stopping the server caused the scrollbars to reappear for the Seedlet theme (but only on Windows). I think this may possibly be due to an issue where the screenshot is captured before the extra CSS to hide the scrollbars is injected into the page.

I wanted to check on the priority of investigating this edge case, as the issue only seems to appear inter intermittently. WDYT?

@wojtekn
Copy link
Contributor

wojtekn commented May 15, 2024

@derekblank, thanks for preparing the fix. I played with multiple sites, using different themes, started and stopped them, and I couldn't reproduce it. Let's merge and close, and we can reopen if we spot it again.

FYI, I was reproducing that not only for the Seedlet theme but for the default one, too.

@wojtekn wojtekn merged commit 1f4b113 into trunk May 15, 2024
13 checks passed
@wojtekn wojtekn deleted the fix/windows-thumbnail-scrollbar branch May 15, 2024 10:14
@derekblank
Copy link
Contributor Author

Let's merge and close, and we can reopen if we spot it again.

Agreed, thanks for taking a look.

Copy link

sentry-io bot commented May 16, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TypeError: Object has been destroyed <anonymous>(main/studio/./src/screenshot-window) View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Platform] Windows The issue is specific to the Windows platform. [Type] Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants