Assessment edit page fixes #2163
Conversation
| errorText: PropTypes.string, | ||
| }; | ||
|
|
||
| export default createComponent( |
There was a problem hiding this comment.
Should we add some basic documentation for this?
There was a problem hiding this comment.
You mean document the create component or ? it was imported from another file. I will add some comments to the second argument.
There was a problem hiding this comment.
I added some detailed documentations to createComponent: https://github.com/Coursemology/coursemology2/pull/2163/files#diff-11a9ab91f987979d52cac0660ae4b0d5R7
Hope this helps for understanding.
There was a problem hiding this comment.
Ah ok - I meant the documentation for the toggle component, like how to use it and what props to pass in. But maybe there is no need to since it seems quite straightforward, what do you think?
There was a problem hiding this comment.
Hmm, I think it's unnecessary because the usage is exactly same as the original material UI component, you can find an example here.
The code here is for integrating with redux-form, so it can be put in Field.
allenwq
force-pushed
the
allen/assessment-edit
branch
2 times, most recently
from
e69a866 to
a46af11
Compare
| import mapError from './mapError'; | ||
|
|
||
| const errorStyle = { | ||
| color: 'red', |
There was a problem hiding this comment.
For consistency, maybe we can import and use red500 from http://www.material-ui.com/#/customization/colors?
| }, | ||
| ...props | ||
| }) => ({ | ||
| // Take our the required fields and send the rest of the props to mapError(). |
| context: { intl }, | ||
| childContextTypes: { intl: intlShape }, | ||
| } | ||
| ).first().shallow(); |
There was a problem hiding this comment.
As discussed, we should .shallow() again and test one level deeper
allenwq
force-pushed
the
allen/assessment-edit
branch
3 times, most recently
from
4aa2b56 to
dadc9a6
Compare
|
Tests added and updated ! |
allenwq
force-pushed
the
allen/assessment-edit
branch
3 times, most recently
from
4640f22 to
75e48fa
Compare
|
Could we add test for Other than that, LGTM! |
|
@kxmbrian How about I test |
|
Yeah, test |
allenwq
force-pushed
the
allen/assessment-edit
branch
from
75e48fa to
96f22b0
Compare
|
@kxmbrian Tests for |
allenwq
force-pushed
the
allen/assessment-edit
branch
from
96f22b0 to
5069782
Compare
|
LGTM - @kxmbrian ? |
kxmbrian
force-pushed
the
allen/assessment-edit
branch
from
3aba80e to
5069782
Compare
Don't merge yet, pending to add tests