New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#352] Started fresh branch to fix #352. Updated component to allow i… #499
Conversation
…nput of zero as default value.
@@ -6,32 +6,23 @@ import SidebarControls from './SidebarControls'; | |||
import DateFormatSelect from './DateFormatSelect'; | |||
|
|||
function DefaultValueInput({ defaultValue, onChange, newType }) { | |||
const emptyValue = newType === 'number' ? 0 : ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gets fed back to DefaultValueInput
as defaultValue
- now that it can be a number, we should probably change the propType of defaultValue
from PropTypes.string
to PropTypes.oneOfType([PropTypes.string, PropTypes.number])
to suppress the console warning when it's 0
(i.e. not a string)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^^ didn't know about this. that's nice. Agreed we should definitely do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would fix the "is not a NaN" console warning messages too, I'm guessing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peeb I think if you wanted to fix the "is not a NaN" console warning, you would have to add an explicit NaN check on line 17 (i.e. (defaultValue !== null && !isNan(defaultValue) ? ...
@gabemart This is clear to re-review when you have time |
lgtm 👍 |
…nput of zero as default value.