feature: migrate to react 16 #141
Conversation
|
@knowbody here's my start. It seems to be working so far. Going to tweak a few more things and see if I can break anything. |
|
Working? The first layout glitch is still there. Even worse. (NB: I clicked once, for the first toggle)
Latest Chrome. |
|
On my computer (and latest Chrome) it works well, that is a strange behavior |
|
Ok I managed to narrow the bug. It appears only when you resize the "Action Logger" until the component window is exactly |
|
@Kerumen Hey, yeah I've seen that before and I think there was an issue at one time about it. Maybe its come back... It looks like an instance of this edge case. Integrating Forto #95 should make this issue go away for good though. Anyways not sure this is an issue particular to this PR. I'll try recreating on master. |
|
@Kerumen I'm not able to reproduce the issue at |
|
@jasonkuhrt Yes the particular |
package.json
Outdated
| "prettier": "^1.7.3", | ||
| "ramda": "^0.18.0", | ||
| "react": "^15.4.2", | ||
| "react-dom": "^15.4.2", | ||
| "react": "16", |
There was a problem hiding this comment.
probably would be better to not use exact v16 and instead use ^16.0.0
There was a problem hiding this comment.
Ah yeah, not used to yarn upgrade@... yet. Shouldn't we make react deps peer deps too?
There was a problem hiding this comment.
There was a problem hiding this comment.
But meh this is still an issue yarnpkg/yarn#2479 so whatever will leave peer deps definition for now.
lib/index.js
Outdated
| } | ||
| constructor (props) { | ||
| super(props) | ||
| this.checkTargetReposition = this.checkTargetReposition.bind(this) |
There was a problem hiding this comment.
if you use arrow functions then you wouldn't need to bind functions in the constructor:
checkTargetReposition = () => {
// ...
}
|
thank you for working on this @jasonkuhrt! |
|
Thanks everyone for your feedback! |
Will close #135.