-
Notifications
You must be signed in to change notification settings - Fork 387
Conversation
Deploy preview ready! Built with commit bbc8561 |
package.json
Outdated
"gh-pages": "^0.12.0", | ||
"glob": "^7.1.1", | ||
"google-map-react": "^0.22.3", | ||
"happo-olivier": "^5.0.2", |
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.
custom fork waiting for the fix to be released.
@oliviertassinari that's super nice! Love it! What do you mean with the task "Test stability"? |
I'm happy to hear it :).
I wanted to make sure the pending http request trick was working with all the stories. That seems to be fine. |
Let's wait for that fix being released then :) |
test/regressions/XMLHttpRequest.js
Outdated
@@ -0,0 +1,17 @@ | |||
(function() { |
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.
Can you add a comment to inform what is this file and why we need it?
Then link to this PR.
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.
Sure
test/regressions/normalize.js
Outdated
@@ -0,0 +1,41 @@ | |||
const path = require('path'); |
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.
Can you add a comment on what is this file doing?
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.
Sure
This looks cool! What would we have to do to get those screenshots/diffs to be ran in multiple browsers? Would having some functional tests going through all stories help? |
test/regressions/normalize.js
Outdated
@@ -0,0 +1,41 @@ | |||
const path = require('path'); |
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.
Needs eslint comment
@vvo Happo doesn't support multiple browsers yet. We would either have to write a chrome-headless target plugin or a selenium target plugin for even more browser coverage like we could go through Browserstack or Saucelab. |
As for screenshots and diffing I would say a ultimate solution would be to have a selenium pluggin that is easy to use yes so that we can take screenshots from many browsers at the same time. Ultimately that's what we would like. |
@oliviertassinari if you wish to be compatible with storybook directly you could create an addon for it. The user will have the ability to use you as a decorator for all their stories which gave you the opportunity to do whatever you want for each on of theme. Check this documentation: https://storybook.js.org/addons/introduction/ |
@mthuret I didn't know about storybook plugins, thanks for sharing it! After going throw the documentation, I don't see how that could help us with the screenshot generation. All we need are the components and the styles when not using a CSS-in-JSS solution. |
@vvo Shouldn't be too hard to write given we have the source code of vrtest doing that, I'm wondering if Happo has any plan for that. |
@@ -32,7 +32,8 @@ function sidebarFollowScroll(sidebarContainer) { | |||
sidebarContainer.style.top = null; | |||
} | |||
sidebarContainer.classList.add('fixed'); | |||
linksContainer.style.maxHeight = `calc(100vh - ${titleHeight + navHeight}px)`; | |||
linksContainer.style.maxHeight = `calc(100vh - ${titleHeight + |
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 have been running yarn reformat
.
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.
Feel free to do so on master, I will rebase if you need a less noisy PR.
LGTM |
@mthuret Let me know if you experience any issue with that solution. I'm here to help. |
This PR is a continuation of algolia/instantsearch#2166.
We are integrating Storybook, Happo and Argos-CI.
The following video is an example of what Happo is doing for us:
![juin-18-2017 00-31-58](https://user-images.githubusercontent.com/3165635/27256687-14a7512a-53be-11e7-8e12-123695968b47.gif)
Things we need to get right:
For more context on the available solutions