-
Notifications
You must be signed in to change notification settings - Fork 843
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
[Cypress] Add EUI's styles to test environment #5371
[Cypress] Add EUI's styles to test environment #5371
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_5371/ |
At a glance (although admittedly I haven't looked deeply) this approach looks solid and scripts/cypress.js LGTM! 👍 I don't see any issues with this overall approach - were there any you were anticipating? |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5371/ |
Pushed up final changes to the
Nope! Just didn't want to go down a rabbit hole with the script + cypress flags if there'd be pushback on the approach :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!skipScss) { | ||
console.log(info('Compiling SCSS')); | ||
execSync( | ||
`TARGET_THEME=amsterdam_dark yarn compile-scss`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any thoughts on allowing us to specify which theme the dev wants to use as an arg? e.g. yarn test-cypress-dev --theme=dark/light
or similar? I'll honestly admit I'm a garbage human being and not a huge fan of dark modes.
No worries either way, this might actually help me test more in dark modes and it's definitely not a blocker!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll honestly admit I'm a garbage human being and not a huge fan of dark modes.
Me too! Good to know there's 2 of us 😆 (and caught up with Greg's comment, a whole 3 of us!), this was the first time I ever reached for dark mode as a default. I like your idea of choosing a theme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a garbage human being and not a huge fan of dark modes
That makes two of us 🙃
I'd say merge as-is and consider theme switching later if it seems useful
I like the theme switch suggestion, as well as the one to merge this as-is. I'll create a follow-up PR with theme selection. |
Summary
Opening as draft because the Cypress arguments section in cypress-testing.md is now broken.
Imports the compiled CSS theme file into Cypress's test environment for more accurate tests.
--skip-css
flag to allow avoiding CSS compilation time--dev
flag to open the Cypress dev environmentChecklist