-
Notifications
You must be signed in to change notification settings - Fork 26
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
feat(select): add isReadOnly prop to select inputs and fields #1192
Conversation
src/components/inputs/async-creatable-select-input/async-creatable-select-input.js
Show resolved
Hide resolved
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.
Ah. It's only styles?
isDisabled: props.isDisabled, | ||
isReadOnly: props.isReadOnly, |
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.
We're now also passing the isDisabled
prop because we will use it for the styles instead of the internal state.isDisabled
, to differentiate between isDisabled
and isReadOnly
.
ff5cd6e
to
e715dec
Compare
Summary
This PR adds the
isReadOnly
prop to ourSelectInput
.It also adds the same prop to
AsyncSelectInput
,CreatableSelectInput
andAsyncCreatableSelectInput
.And then also to their 4 corresponding
*SelectField
components.There's a lot changes because it's the same change in 8 different components, but the focus of the review can be on only one of them and on the
create-select-styles
, which is shared across all of them.Description
react-select
doesn't supportisReadOnly
because it doesn't have any functional difference fromisDisabled
, it's only a matter of styling it differently. So basically I'm disabling it internally and giving it the sameread-only
visuals as we have for the other inputs.But the main point of adding the prop is because almost all of our inputs support
readOnly
and users of the library may erroneously expect it for this component as well, as it happened here. So it's a matter of consistency.