-
-
Notifications
You must be signed in to change notification settings - Fork 173
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
Add option of specifying media features for chrome #388
Add option of specifying media features for chrome #388
Conversation
Reference images for the new stories are missing |
@techeverri Yeah, I mentioned in the description that I wasn't able to generate them due to |
Storybook compiles normally, but all stories from
Which is thrown in browser console Note. after commenting out usage of |
Yeah, these are deprecated APIs and I guess it's time to remove them from the tests too (I kept them to not break accidentally). They run fine on master though and looking at the test output, it complains about reference image not being present (you added new stories and configurations without adding baselines, so that is kinda expected). If you're unable to run the tests locally (try rebasing in that case) you can also download the baselines from the test run: https://github.com/oblador/loki/suites/6373156022/artifacts/234968222 |
Seems like the baseline is not actually correct. The tests run fine, but all examples from the above mentioned file (
So should I delete the usage of those APIs from examples and update baseline images as part of this PR or do we need this work done on separate PR? |
I can do this in a separate PR 👍 |
Fixed in #390! |
Sorry for being annoying here, but don't you think it would enough to do this for just one test instead of adding two more configurations? See this example: https://github.com/oblador/loki/blob/master/examples/react/src/api/Decorators.stories.js#L56 |
This is actually a brilliant idea 👍 I had to update chrome target to allow passing of story parameters to @oblador is there any reason why it wasn't done previously? - ff69659 |
Ah, didn't know this wasn't supported yet :D Thanks for fixing/adding! |
| **`skipStories`** | _string_ | **DEPRECATED** Same as `loki.skipStories`, but applied to only this configuration. | All | | ||
| **`storiesFilter`** | _string_ | Same as `loki.storiesFilter`, but applied to only this configuration. | All | | ||
| **`chromeSelector`** | _string_ | Same as `loki.chromeSelector`, but applied to only this configuration. | `chrome.*` | | ||
| **`preset`** | _string_ | Predefined bundled configuration, possible values are `Retina Macbook Pro 15`, `Retina Macbook Pro 15 Dark Mode`, `iPhone 7`, `iPhone 7 Dark Mode`, `iPhone 5`, `iPhone 5 Dark Mode`, `Google Pixel`, `Google Pixel Dark Mode`, `A4 Paper`, and `US Letter Paper`. | `chrome.*` | |
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.
We should remove this from the docs before merging
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.
Never mind I just saw that it was added to the preset file https://github.com/oblador/loki/pull/388/files#diff-32dd6d5e208ff82474b1c3cfc1bd3b9ee59482fb9939ca0117de04a70eb4a4a3
This feature adds support for emulating specific CSS media features (like preferred color scheme). The features parameter of
setEmulatedMedia
can be specified via configuration file for any chrome-based target.It is inspired by the idea proposed by @oblador's idea to override what users can emulate in this commment. It also was mentioned in the following discussion.
The PR also updates the application in the
examples
folder, but at the current time the configuration of it seems to be broken due to:loki/examples/react/src/stories/index.stories.js
Line 24 in 0e7acbd
loki/examples/react/src/stories/index.stories.js
Line 36 in 0e7acbd
loki specific API usage, that seems to be misconfigured/deprecated. @oblador @techeverri if you could please provide some guidance on how to fix these issues, I would be more than happy to include it in this PR (together with updated loki's reference images)