Skip to content

Add support for dark mode screenshots - #15010

Merged
startuptester merged 5 commits into
developfrom
add/screenshot-dark-mode
Oct 12, 2020
Merged

Add support for dark mode screenshots#15010
startuptester merged 5 commits into
developfrom
add/screenshot-dark-mode

Conversation

@rachelmcr

@rachelmcr rachelmcr commented Sep 29, 2020

Copy link
Copy Markdown
Contributor

Adds dark mode screenshots and updates the Fastlane SnapshotHelper.

Changes

The UI tests now support setting the simulator to dark mode, and the Fastlane screenshots lane runs through the screenshots twice — once in dark mode and once in light mode. The screenshot test also uses a new helper method to include light/dark mode in the filename.

(These changes match the approach used in the WooCommerce iOS screenshots to support light/dark mode, h/t @jkmassel for that implementation.)

The SnapshotHelper update resolves an issue with the iPad landscape screenshots (part of the 2.159.0 Fastlane release). Previously those screenshots would appear incorrectly rotated even when the simulator was in landscape orientation.

To test

  1. Open the fastlane directory: cd Scripts/fastlane
  2. Generate the screenshots: bundle exec fastlane screenshots (or bundle exec fastlane screenshots language:"en-US" to run it for a single language — the full run can take a long time and there aren't language-specific changes.)
  3. Confirm there’s now a full set of screenshots in both light & dark mode, and iPad screenshots have the correct landscape orientation.

These generated screenshots will also come in handy for reviewing an upcoming PR (changing the app store screenshot composition).

To review

This PR only requires a single reviewer.

PR submission checklist

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@rachelmcr rachelmcr added the Tooling Build, Release, and Validation Tools label Sep 29, 2020
@rachelmcr rachelmcr added this to the 15.9 milestone Sep 29, 2020
@peril-wordpress-mobile

peril-wordpress-mobile Bot commented Sep 29, 2020

Copy link
Copy Markdown

You can trigger an installable build for these changes by visiting CircleCI here.

@peril-wordpress-mobile

peril-wordpress-mobile Bot commented Sep 29, 2020

Copy link
Copy Markdown

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@rachelmcr

Copy link
Copy Markdown
Contributor Author

Let's hold off on reviewing/merging this one, so we can get the initial changes to the screenshot test done first (to support the new screenshot designs). I'll update this PR once those changes are done and we can take a look at it then.

@jkmassel

jkmassel commented Oct 5, 2020

Copy link
Copy Markdown
Contributor

👋 We're freezing 15.9 today, so I'm bumping this PR forward to 16.0. If you need it to be part of the 15.9 release cycle, please merge to the release/15.9 branch and DM me – I'll be happy to cut a new beta release!

@jkmassel jkmassel modified the milestones: 15.9, 16.0 Oct 5, 2020
@startuptester

Copy link
Copy Markdown
Contributor

@jkmassel maybe I misunderstand, but I am pretty sure this does need to be in 15.9.

@jkmassel

jkmassel commented Oct 5, 2020

Copy link
Copy Markdown
Contributor

No problem at all – just means that it needs to be merged into release/15.9 instead of develop when it's ready! 😃

@rachelmcr

Copy link
Copy Markdown
Contributor Author

@startuptester this is actually ready for review now! I left it targeting develop because even though we plan to generate new screenshots for the 15.9 release, we don't expect any significant UI differences between the release/15.9 and develop branches.

@rachelmcr

Copy link
Copy Markdown
Contributor Author

While generating a full set of screenshots I discovered that they're not all being generated — nothing is generated for th or ru because the test fails consistently in those languages. This is an issue with a full set of screenshots I generated before this branch, as well, so it isn't specific to this PR. (I didn't notice earlier because those languages don't appear at all in screenshots.html; you have to check each language in the screenshots/ directory to notice some are missing.)

Since this isn't specific to this PR, I opened #15074 to follow up on those failures.

@startuptester startuptester left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Russian and Thai are empty as you noted but
Spanish is the only language with both light and dark screenshots, and the light ones are not rotated. (see attached image)
Screen Shot 2020-10-09 at 4 33 32 PM

@rachelmcr

Copy link
Copy Markdown
Contributor Author

You ran the script in Spanish while testing #15055, right? Did you clear out those Spanish screenshots before testing this PR? Since this PR includes a change that doesn't automatically clear previous screenshots, those are likely copies from that previous run (which had an older version of Fastlane without the orientation fix).

@startuptester

startuptester commented Oct 11, 2020

Copy link
Copy Markdown
Contributor

You ran the script in Spanish while testing #15055, right? Did you clear out those Spanish screenshots before testing this PR?

Ah, genius! But I also ran this before with no specified language and while those screenshots are still stored, they aren't in the .html..? Just to make sure, I'll clear everything out and run again now.

@startuptester startuptester left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All good! 👍

@startuptester
startuptester merged commit db25e6b into develop Oct 12, 2020
@startuptester
startuptester deleted the add/screenshot-dark-mode branch October 12, 2020 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Tooling Build, Release, and Validation Tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants