Skip to content

Conversation

@isoos
Copy link
Collaborator

@isoos isoos commented Feb 19, 2025

  • Automatically takes separate screenshots for typical mobile, tablet and desktop widths and for dark and light themes (6 combinations total).
  • Starting with 11 locations (66 screenshots), some full-page, some per-component screenshots.
  • Comparison tool and github CI integrations in subsequent PRs.

final _themes = ['dark', 'light'];
final _viewports = {
'mobile': DeviceViewport(width: 400, height: 800),
'tablet': DeviceViewport(width: 768, height: 1024),
Copy link
Member

Choose a reason for hiding this comment

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

Might I propose we don't do tablet (just because of the number of screenshots)? Also don't those normally have much higher resolution (same with phones).

I'm assuming this is more about aspect ratio?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why don't do tablet? I think it is a good way to check how the site behaves in narrower screen, and tbh. I don't think we will manually check tablet screens, but may catch if something degrades.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm assuming this is more about aspect ratio?

Mostly the width.

Copy link
Member

Choose a reason for hiding this comment

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

I'm mostly thinking that we already have a LOT of screenshots, so I don't mind it, I'm only concerned that it's hard to get an overview.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's keep them for now, in the hope they are useful. If they are not, create too much noise, we may remove them or ignore them in most comparison reports.

// Default screen with 16:10 ratio.
final desktopDeviceViewport = DeviceViewport(width: 1280, height: 800);

final _screenshotDir = Platform.environment['PUB_SCREENSHOT_DIR'];
Copy link
Member

Choose a reason for hiding this comment

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

How about we just always store the screenshots in .dart_tool/screenshots/.

  • The .dart_tool/ folder is already there.
  • The .dart_tool/ folder is always in .gitignore.

Copy link
Member

Choose a reason for hiding this comment

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

Borderline crazy idea:

.dart_tool/
  screenshots/
    <revision>/
      image1.png
    current/
      image1.png
    comparison.html

Whenever, we create screenshots we:

  • Always write to .dart_tool/screenshots/current/
  • If git status --porcelain (meaning we have no changes), then:
    • We also write to .dart_tool/screenshots/<revision>/
  • We always create .dart_tool/screenshots/comparison.html
    • A simple dump HTML file showing screenshots side by side
    • Contents of .dart_tool/screenshots/current/ on the right side
    • Contents of <revision> on the left side, where you pick <revision> from a dropdown
      • We create a dropdown at the top of the HTML file.
      • We run git log to find relevant revisions
        • We put relevant revisions in dropdown, if .dart_tool/screenshots/<revision>/ exists locally.
        • We order the dropdown by git log, and default to the most recent revision we have.

We could also use gh api to download artifacts from github:
https://docs.github.com/en/rest/actions/artifacts?apiVersion=2022-11-28#get-an-artifact

Note

  • An artifact on github is a zip folder.
  • It's not possible to download an artifact without being authenticated!
  • Artifacts can't be embedded in comments as images :(

If wanted to have before/after images on github, we'll probably have to do something borderline crazy like uploading the images to a separate branch or a GCS bucket.
Which probably involve more infrastructure than we care to stand up and maintain.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not a crazy idea, but let's not rush to it yet, and reach it maybe step-by-step. There is an immediate benefit with this + a comparison tool, and then we can add further automatization with github artifacts and/or per-revision storage...

@isoos isoos merged commit a0c4ca1 into dart-lang:master Feb 20, 2025
31 checks passed
@isoos isoos deleted the screenshots branch February 20, 2025 10:56
isoos added a commit to isoos/pub-dev that referenced this pull request Mar 5, 2025
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.

3 participants