-
Notifications
You must be signed in to change notification settings - Fork 609
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
Scenario Specific Viewports #844
Conversation
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.
Yes. Looks good!
A few comments.
-
please add a test with an empty scenario.viewports array
[]
. I think this case should fall back to the root viewports array. -
please use unique label and dimension values in your
scenarioSpecificViewports
test. -
Please add an additional test that specifies four viewports with unique values.
-
Please add an additional test that uses the four viewports from above but also uses the
expandSelector: true
prop. -
please think of any other potential property/feature conflict and add a test for that as well 😉
In such case viewports that will fallback to the defined ones in the configuration.
Default separator for the defining the scenarios is underscore. Using dash will prevent any confusion.
Thanks for this PR! I don't have enough time to publish this today but this will definitely go out in the next release. |
Pushed to NPM. V3.5.8 |
@garris Hello, thanks for merging and releasing this so fast! |
Hello again @garris , I tried to use the changes today, but I couldn't find the changes in the code. I can't see changes also in unpkg; https://unpkg.com/[email protected]/core/util/createBitmaps.js Am I doing something wrong? |
🤔 Ok, I’ll try to check this today. |
Ok, apologies. My bad. Please validate using v3.5.9. Thanks! |
It's working now, thanks for your help all along! 🎊 |
This PR will provide scenario specific viewports. Previously, tests were executing accordingly to viewports provided by configuration. With this PR, global viewports will be able to overrided by the scenario specific ones.
For more information see #472