fix: CORUI-6120: Add required and default props #322
Conversation
|
there are a number of props that moved to required here - it would be good to kick out a new release to get updated docs to go along with this PR. |
src/Forms/Forms.js
Outdated
| children: PropTypes.node, | ||
| className: PropTypes.string, | ||
| type: PropTypes.oneOf(['', 'error', 'warning', 'help']) | ||
| type: PropTypes.oneOf(['error', 'warning', 'help']) |
There was a problem hiding this comment.
@mobot11 : looks like this was missed in the earlier const PR
There was a problem hiding this comment.
We should either fix this or create a follow-up issue.
|
really nice cleanup here, @cjskillingstad! |
src/Forms/Forms.js
Outdated
| children: PropTypes.node, | ||
| className: PropTypes.string, | ||
| type: PropTypes.oneOf(['', 'error', 'warning', 'help']) | ||
| type: PropTypes.oneOf(['error', 'warning', 'help']) |
There was a problem hiding this comment.
We should either fix this or create a follow-up issue.
| // check to make sure itemsPerPage != 0 | ||
| this.numberOfPages = Math.ceil( | ||
| itemsTotal / (itemsPerPage ? itemsPerPage : 10) | ||
| itemsTotal / (itemsPerPage || 10) |
There was a problem hiding this comment.
With a default value, we could remove the || 10.
There was a problem hiding this comment.
I thought about this, but if the consumer passes in a 0 then this will break
There was a problem hiding this comment.
Fair enough. If it would break with certain values, then we should consider adding a custom prop type validator that would prevent that. But, that would be a follow-up enhancement if we wanted to do that.
|
@mobot11 @greg-a-smith Using const's for the last couple places we missed in the propTypes in e09cb71 |
greg-a-smith
left a comment
greg-a-smith
left a comment
There was a problem hiding this comment.
Looks good. 🚢 Lots of work, but worth it in the long run!
* ActionBar * Alert * Button * Calendar * DatePicker * Dropdown * FormRadio * InputGroup * LocalizationEditor * Menu * Modal * MultiInput * Pagination * SearchInput * Shellbar * SideNavigation * Table * Tabs * Tile * Tile * Time * TimePicker * Toggle * Remove false default props * Make description optional * Add default prop for funcs * Default prop consts * Remove static proptypes * Remove last false default prop
Description
'', funcs as() => {}, numbers asnull, objects as{}, etc. as it seemed somewhat unnecessary. I did do the bools as false because I think it will make the documentation more readable. Let me know if you think I should do those other cases.fixes #244