Removes the magic in Input, reducing it to a styled TextField #150
Conversation
rileyjbauer
force-pushed
the
simplify-inputs
branch
from
644f5e3 to
e6da196
Compare
|
|
||
| {experimentName && ( | ||
| <div> | ||
| <div>This run will be associated with the following experiment</div> | ||
| <Input label='Experiment' instance={this} disabled={true} field='experimentName' /> | ||
| <Input label='Experiment' onChange={this.handleChange('experimentName')} | ||
| disabled={true} value={experimentName} /> | ||
| </div> | ||
| )} | ||
|
|
There was a problem hiding this comment.
Down below on line 177, we're using a plain TextField, and I think the reason is because we wanted a custom onChange handler, so it makes sense to make that an Input now.
There was a problem hiding this comment.
I'll make this change in the branch where I'm working on NewRun tests
frontend/src/atoms/Input.tsx
Outdated
| const { height, width, ...rest } = props; | ||
| return ( | ||
| <TextField variant='outlined' className={commonCss.textField} spellCheck={false} | ||
| style={{ height: height || 40, maxWidth: 600, width: width || '100%' }} {...rest}> |
There was a problem hiding this comment.
Does it make sense to set height: 'auto' automatically if the prop multiline === true?
There was a problem hiding this comment.
Yes, I believe so.
|
@rileyjbauer: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
rileyjbauer
force-pushed
the
simplify-inputs
branch
from
69e36a9 to
b72ee33
Compare
|
/retest |
rileyjbauer
force-pushed
the
simplify-inputs
branch
from
b72ee33 to
a3f01c6
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yebrahim The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yebrahim The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
I saw all tests finish successfully on this. For a few minutes, I could see Github's three green buttons here and could merge manually, then saw this test failure. @IronPan any ideas what's happening here? |
|
/retest |
1 similar comment
|
/retest |
* Fix test error by golint * Enable travis to make test * Keep mini change
Inputpreviously took its parent's instance as a prop in order to automatically update the field in its parent's state using just the field name, however, this caused an additional copy of the instance to be made for eachInputelement in the parent.This change is