-
Notifications
You must be signed in to change notification settings - Fork 334
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 specs for history slider #1053
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.
Very nice! Just one minor comment.
@@ -0,0 +1,47 @@ | |||
/* @flow */ |
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 you remove this line, CI will go green. Otherwise we'd need to configure flow
properly.
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've noticed that and did a second commit on my repo. Do you know why it doesn't show up in the pull request?
Edit: It did now, perhaps it just takes time
Enzyme.configure({ adapter: new Adapter() }); | ||
|
||
import store from "../../../lib/store"; | ||
import Kernel from "../../../lib/kernel"; |
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 think those lines can be removed too:
import store from "../../../lib/store";
import Kernel from "../../../lib/kernel";
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.
Looks great, keep those PRs coming 🎉
This is a rudimentary set of tests for the history slider. (Just testing basic rendering and the arrow buttons)
Ref #813