Move LoginModal username and password into instance properties, remove refs #966
Conversation
| @@ -9,17 +9,16 @@ export default class LoginModal extends React.Component { | |||
| constructor(props) { | |||
| super(props); | |||
| this.handleSubmit = this.handleSubmit.bind(this); | |||
| this.usernameRef = React.createRef(); | |||
| this.passwordRef = React.createRef(); | |||
| this.state = { | |||
There was a problem hiding this comment.
There is no need to store this values in the component state, they can be class properties
this.username and this.password
There was a problem hiding this comment.
Thanks for the feedback! I don't have any React experience, so it's much appreciated. I've replaced the state two with properties.
| @@ -54,7 +53,10 @@ export default class LoginModal extends React.Component { | |||
| <FormControl | |||
| type='text' | |||
| placeholder='User Name' | |||
| ref={this.usernameRef} | |||
| value={this.state.username} | |||
There was a problem hiding this comment.
the FormControl does not have to be re-rendered every time there is a key stroke
There was a problem hiding this comment.
Thanks, removed
| @@ -54,7 +53,10 @@ export default class LoginModal extends React.Component { | |||
| <FormControl | |||
| type='text' | |||
| placeholder='User Name' | |||
| ref={this.usernameRef} | |||
| value={this.state.username} | |||
| onChange={(e) => { | |||
There was a problem hiding this comment.
lets make this a function in the class something like
setUser = (e) => { this.username = e.target.value }then the onChange prop is onChange=this.setUser
| @@ -70,7 +72,10 @@ export default class LoginModal extends React.Component { | |||
| <FormControl | |||
| type='password' | |||
| placeholder='Password' | |||
| ref={this.passwordRef} | |||
| value={this.state.password} | |||
| onChange={(e) => { | |||
There was a problem hiding this comment.
see above comment about the function
…es into instance properties instead
MikeNicholls
force-pushed
the
login-modal-fix
branch
from
22964f1 to
0bcaf3c
Compare
MikeNicholls
changed the title
|
Not sure which dev is owning reviews on this, assigning one more reviewer. |
deanwhillier
added
4: Reviews Complete
There was a problem hiding this comment.
Working completely fine.
Tested on the master branch rebased with this PR on Windows 10 and Linux.
Tests passed on Windows 10 but are still failing on Linux: https://gist.github.com/wget/b02f11e270f25244e405bef690c975e8
…sic Auth Remove use of refs for login dialog username and password, shift values into instance properties instead (mattermost#966)
Before submitting, please confirm you've
npm run lint:jsfor proper code formattingPlease provide the following information:
Summary
Resolves an issue where username and password were not being passed for HTTP basic authentication.
Issue link
Fixes #965
Test Cases
Additional Notes