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

Change default theme jest config platform from iOS -> win32 #2500

Merged
merged 3 commits into from
Jan 9, 2023

Conversation

lyzhan7
Copy link
Contributor

@lyzhan7 lyzhan7 commented Jan 9, 2023

Platforms Impacted

  • iOS
  • macOS
  • win32 (Office)
  • windows
  • android

Description of changes

This PR is related to #2481, decided to make this a separate PR in order to keep the original as small as possible.

Why this change is required for PR 2481:

  • Currently the default-theme tests run on iOS, and before PR 2481 goes in iOS will have been using web/windows alias tokens. So technically, the default-theme tests were never really testing iOS, they've been generally testing web/windows theme creation. Also, the default-theme tests include tests for high contrast mode and 'dynamic' mode - neither of these exist on iOS yet.
  • After PR 2481 iOS will be using iOS alias tokens. This means the default-theme tests will be using iOS alias tokens. It doesn't make sense to be running high contrast tests on iOS, so updating these tests to actually be running on a platform that has high contrast
  • Note - these tests still do run on iOS, I had to make a change in PR 2481 so that on iOS, if we try to access high contrast mode, instead of throwing an error we return the light mode tokens. I could keep the platform here set to iOS, just the high contrast mode tests will be really testing light mode tokens

For more context behind the current state of the default theme see #2492

Verification

No visual changes

Pull request checklist

This PR has considered (when applicable):

  • Automated Tests
  • Documentation and examples
  • Keyboard Accessibility
  • Voiceover
  • Internationalization and Right-to-left Layouts

@lyzhan7 lyzhan7 requested a review from a team as a code owner January 9, 2023 18:54
@lyzhan7 lyzhan7 changed the title Change default theme jest config Change default theme jest config platform from iOS -> win32 Jan 9, 2023
@lyzhan7
Copy link
Contributor Author

lyzhan7 commented Jan 9, 2023

I realized another option is to change the platform to windows instead of win32, maybe this would make more sense since the default-theme was created with with web/windows tokens in mind? Not sure what the difference is between web/windows and win32 tokens. Thinking I'll leave it as is for now though since win32 is used more often & might be better to have more test coverage on win32

@lyzhan7 lyzhan7 merged commit 17bf1b5 into microsoft:main Jan 9, 2023
@lyzhan7 lyzhan7 deleted the change-default-theme-jest-config branch January 9, 2023 21:57
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