React/dp 12935 empty divs #510
Conversation
| } | ||
| } | ||
| InputSync.propTypes = { | ||
| formIds: PropTypes.arrayOf(PropTypes.string).isRequired, |
There was a problem hiding this comment.
add comments above these so they are rendered in documentation
| } | ||
| } | ||
| InputLink.propTypes = { | ||
| formIds: PropTypes.arrayOf(PropTypes.string).isRequired, |
There was a problem hiding this comment.
add comments above these so they are rendered in documentation
| const Form = (props) => { | ||
| const formContext = useContext(FormContext); | ||
| return( | ||
| <form className="ma__form-page" action="#"> |
There was a problem hiding this comment.
just curious - i know this was there before - should the action be a passable prop with a default of #?
There was a problem hiding this comment.
It should yes!
| // Update the inputs back to percentages when slider changes value. | ||
| inputSliderOptionsWithKnobs.updateFunc = (val) => { | ||
| const newVal = Number(numbro(val * 100).format({ mantissa: 0 })); | ||
| // We need to ensure that the new percentage value isn't already set on the Inputs we are changing. |
There was a problem hiding this comment.
what do you mean by "isn't already set" here?
| @@ -24,9 +23,46 @@ class FormProvider extends Component { | |||
| hasId: this.hasId, | |||
| setValue: this.setValue, | |||
| updateState: this.updateState, | |||
| getValues: this.getValues | |||
| getValues: this.getValues, | |||
There was a problem hiding this comment.
I wonder if these methods need to be in the state or can directly be passed to Provider value?
There was a problem hiding this comment.
e.g.
| getValues: this.getValues, | |
| this.methods = { | |
| getValue: this.getValue, | |
| hasId: this.hasId, | |
| setValue: this.setValue, | |
| updateState: this.updateState, | |
| getValues: this.getValues, | |
| syncContent: this.syncContent, | |
| getLinkedContent: this.getLinkedContent, | |
| updateLinkedContent: this.updateLinkedContent, | |
| setLinkedContent: this.setLinkedContent, | |
| onUpdate: this.onUpdate, | |
| getUpdateFunc: this.getUpdateFunc, | |
| setUpdateFunc: this.setUpdateFunc | |
| } |
| @@ -0,0 +1,63 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
add a story about this with documentation and use example
| @@ -0,0 +1,79 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
add a story about this with documentation and use example
There was a problem hiding this comment.
this is used in Form story
| if (is.fn(this.context.syncContent)) { | ||
| this.context.syncContent(this.props.id); | ||
| } | ||
| } | ||
| getValue = () => this.state.value; | ||
| setValue = (value, afterUpdate) => { |
There was a problem hiding this comment.
how is afterUpdate being used? can you add a description to this param?
| @@ -36,19 +40,72 @@ class InputProvider extends React.Component { | |||
| super(props); | |||
| this.state = { | |||
| value: this.props.defaultValue, | |||
| linkedContent: this.props.linkedContent, | |||
There was a problem hiding this comment.
can you add propTypes for Input and document all the props that Input/InputProvider expect?
| // syncCondition is ran for each id passed in props.formIds from the Form component. | ||
| // This controls what happens for a single call to syncCondition. | ||
| // syncCondition overrides the default behavior in the else below. | ||
| if (is.fn(this.props.syncCondition)) { |
There was a problem hiding this comment.
can you add syncCondition to propTypes and add description
| formIds: PropTypes.arrayOf(PropTypes.string).isRequired, | ||
| children: PropTypes.any | ||
| }; | ||
| InputSync.contextType = FormContext; |
There was a problem hiding this comment.
Can we use useContext here?
There was a problem hiding this comment.
oh nvm, can it only be used in functional component?
| componentDidMount() { | ||
| this.checkFormContext(); | ||
| } | ||
| checkFormContext = () => { |
There was a problem hiding this comment.
Can you add comment about what checkFormContext is doing, and also in Input there's also checkFormContext but they are doing different things.
| const { value } = this.context; | ||
| value[id].syncContent = []; | ||
| value[id].syncContent.push(this.syncContent); | ||
| this.context.updateState({ value }); |
There was a problem hiding this comment.
seems like this is updating Form context value? is it able to update the value if so can you provide an example?
There was a problem hiding this comment.
ok got it this is pushing the syncContent function array to value in formContext so that if can be used to update its own state
| getUpdateFunc: this.getUpdateFunc, | ||
| setUpdateFunc: this.setUpdateFunc, | ||
| getOverrideLink: this.getOverrideLink | ||
| }; | ||
| formContext.updateState({ value }); |
There was a problem hiding this comment.
what's the purpose of updating form context methods from Input?
| showError, | ||
| errorMsg, | ||
| value: toCurrency(newValue, countDecimals(props.step)) | ||
| errorMsg | ||
| }, () => { | ||
| if (typeof props.onChange === 'function') { |
There was a problem hiding this comment.
| if (typeof props.onChange === 'function') { | |
| if (props.onChange && is.fn(props.onChange)) { |
| getValue = () => this.state.value; | ||
| setValue = (value, afterUpdate) => { | ||
| this.setState({ value }, afterUpdate); | ||
| getOwnRef = () => this.selfRef; |
There was a problem hiding this comment.
how is this being used?
Co-Authored-By: smurrayatwork <[email protected]>
Co-Authored-By: smurrayatwork <[email protected]>
Co-Authored-By: smurrayatwork <[email protected]>
Co-Authored-By: smurrayatwork <[email protected]>
…ower into react/DP-12935--empty-divs
Any PRs being created needs a changelog.txt file before being merged into dev. See: Change Log Instructions
Description
Related Issue / Ticket
Steps to Test
Screenshots
Use something like licecap to capture gifs to demonstrate behaviors.
Additional Notes:
Anything else to add?
Impacted Areas in Application
@todo
Today I learned...