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

[all] Adding support for and styling readOnly fields. #440 #574

Merged
merged 8 commits into from
Feb 16, 2018

Conversation

kristofferjs
Copy link
Contributor

Not ready to merge yet.
Needs to pass down readOnly to children of array and objects. (@bjoerge)

Copy link
Member

@bjoerge bjoerge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got a few comments, apart from that this looks very good

@@ -45,7 +45,7 @@ export default class RadioSelect extends React.Component {
}

render() {
const {items, value, name, direction} = this.props
const {items, value, name, direction, readOnly} = this.props
Copy link
Member

@bjoerge bjoerge Feb 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readOnly is missing in props validation

@@ -174,13 +177,13 @@ export default class StatelessSearchableSelect extends React.PureComponent {
)
}
{
onClear && value && (
onClear && value && !this.props.readOnly && (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to destruct readOnly along with the rest of the props

<button type="button" className={styles.clearButton} onClick={onClear}>
<CloseIcon color="inherit" />
</button>
)
}
{!isLoading && (
{!isLoading && !this.props.readOnly && (
Copy link
Member

@bjoerge bjoerge Feb 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access the destructed variable here too

@bjoerge
Copy link
Member

bjoerge commented Feb 15, 2018

I think this is good to merge now. I've made sure the readOnly prop gets passed down from container input components to their members, and also improved how the image and file inputs renders when readOnly.

@kristofferjs kristofferjs merged commit c716c93 into next Feb 16, 2018
@kristofferjs kristofferjs deleted the fix/read-only-inputs branch February 16, 2018 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants