Skip to content
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

Merged
merged 1 commit into from
Nov 25, 2019

Conversation

jonnybel
Copy link
Contributor

@jonnybel jonnybel commented Nov 21, 2019

Summary

This PR adds the isReadOnly prop to our SelectInput.
It also adds the same prop to AsyncSelectInput, CreatableSelectInput and AsyncCreatableSelectInput.
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 support isReadOnly because it doesn't have any functional difference from isDisabled, it's only a matter of styling it differently. So basically I'm disabling it internally and giving it the same read-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.

@jonnybel jonnybel self-assigned this Nov 21, 2019
Copy link
Contributor

@tdeekens tdeekens left a 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?

Comment on lines +50 to +51
isDisabled: props.isDisabled,
isReadOnly: props.isReadOnly,
Copy link
Contributor Author

@jonnybel jonnybel Nov 21, 2019

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.

@jonnybel jonnybel requested a review from montezume November 21, 2019 17:07
@montezume montezume force-pushed the ja-select-input-readonly branch from ff5cd6e to e715dec Compare November 25, 2019 10:54
@kodiakhq kodiakhq bot merged commit cbf0f3e into master Nov 25, 2019
@kodiakhq kodiakhq bot deleted the ja-select-input-readonly branch November 25, 2019 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants