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
Change examples from class
to functional
components
#5393
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 4a427ac:
|
46aeb8d
to
2cf3aa2
Compare
@@ -22,24 +18,6 @@ const loadOptions = ( | |||
}, 1000); | |||
}; | |||
|
|||
export default class WithCallbacks extends Component<{}, State> { | |||
state: State = { inputValue: '' }; |
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.
I've removed the state logic to simplify the example. I don't think it was necessary for demonstrating how to use the async callbacks api.
console.group('Value Changed'); | ||
console.log(newValue); | ||
console.log(`action: ${actionMeta.action}`); | ||
console.groupEnd(); |
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.
I've removed the console logs to simplify the example. I don't think they were necessary for the purpose of this example.
@@ -20,20 +16,10 @@ const promiseOptions = (inputValue: string) => | |||
}, 1000); | |||
}); | |||
|
|||
export default class WithPromises extends Component<{}, State> { | |||
state: State = { inputValue: '' }; |
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.
I've removed the state logic from this example because it wasn't necessary for the purpose of this example.
}; | ||
render() { | ||
const { value } = this.state; | ||
const displayValue = |
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.
I've removed this to simplify the example.
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.
I kinda like it for this example ¯_(ツ)_/¯
value.value.toString()
is a bit grim though — maybe const [dateOption, setDateOption] = useState<DateOption | null>(defaultOptions[0] as DateOption)
would be better?
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.
🤔 Hm. Isn't that what i'm already doing in line 222 (except i've just left it named as value
instead of renaming it to dateOption
)?
handleInputChange = (newValue: string) => { | ||
const inputValue = newValue.replace(/\W/g, ''); | ||
this.setState({ inputValue }); | ||
return inputValue; |
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.
I've removed this because it wasn't being used
docs/examples/BasicSingle.tsx
Outdated
readonly isSearchable: boolean; | ||
} | ||
export default () => { | ||
const [isClearable, setIsClearable] = useState(false); |
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.
isClearable
and isSearchable
should start off as true
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.
good catch 👍
isMulti={false} | ||
isOptionSelected={(o, v) => v.some((i) => i.date.isSame(o.date, 'day'))} | ||
maxMenuHeight={380} | ||
onChange={props.onChange} |
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 is spread on the props that are spread in above
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 is how it was in the original example. I'm just gonna leave it the way it was. Personally, i think the props spread should go and the onChange
should stay (it's more explicit), but im too scared to delete the props spread in case i break something 😰
onChange={props.onChange} | ||
onInputChange={handleInputChange} | ||
options={options} | ||
value={props.value} |
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 is spread on the props that are spread in above
This PR updates the examples to use
functional
components