Room header and copy link #34
Conversation
| @@ -9,13 +9,13 @@ class Join extends Component { | |||
| }; | |||
| } | |||
|
|
|||
| handleChange = (e) => { | |||
| handleChange = (e) => { // eslint-disable-line | |||
There was a problem hiding this comment.
In this way, we will always add eslint-disable-line to bypass this line.
If you don't like .bind(this), I think we can move () => into component like onSubmit={() => this.handleSubmit()}.
There was a problem hiding this comment.
This is false-positive, and strangely it doesn't happen every time or on every machine, please check out the issue I mentioned in the PR description. This is a bug of eslint or babel-eslint and it should be fixed in the future.
As for workarounds, there are many of them, including binding them manually in constructor. Actually the method you gave looks less appealing than that, and it seems won't work. handleSubmit is still not bound to the class context. I'll try this when I got time.
src/components/Share/Share.js
Outdated
| } | ||
| } | ||
|
|
||
| const defaultTooltip = 'Click to copy link'; |
There was a problem hiding this comment.
Let's move the const to the top, then we can see it easily.
There was a problem hiding this comment.
Hmm... I put it at the bottom because this is how React Native Tutorial done with the style declaration. I'm okay with either place, top or bottom.
There was a problem hiding this comment.
Yeah, let's put it to the top as we usually did like this in FE routine work.
|
|
||
| input { | ||
| position: absolute; | ||
| left: -99999px; |
There was a problem hiding this comment.
I know why you mentioned it's hacky now...
Could we use other hacky way instead of setting the left to a strange value?
There was a problem hiding this comment.
I'll try those ways we discussed about, but frankly, they look pretty much the same to me. 😉
There was a problem hiding this comment.
OK, not a big problem, just think -99999px is not good looking.
There was a problem hiding this comment.
<input> needs to be visible for this trick to work, and by visible it means it cannot be display: none or visibility: hidden. opacity: 0 and/or width: 0; height: 0 works, but does not look any less hacky. Actually, kicking elements off screen like this is a common trick used in front end development for years.
This resolves #25.
eslint-disable-line: There's a no-undef false positive from eslint, see also: Build errorno-undeffor flowtypes with class properties facebook/create-react-app#2604