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

Allow edits to be rejected for EditableCell #2192

Closed
wants to merge 2 commits into from
Closed

Allow edits to be rejected for EditableCell #2192

wants to merge 2 commits into from

Conversation

maclockard
Copy link
Contributor

@maclockard maclockard commented Mar 2, 2018

Checklist

Changes proposed in this pull request:

Currently, if I don't change the controlled value as the result of a onConfirm, this check prevents the original value from overriding the savedValue. This is useful in a case where input to the EditableCell is being validated, and if it doesn't pass validation, the value of the cell shouldn't change.

Currently, if I don't change the controlled value as the result of a `onConfirm`, this check prevents the original value from overriding the savedValue. This is useful in a case where input to the `EditableCell` is being validated, and if it doesn't pass validation, the value of the cell shouldn't change.
@palantirtech
Copy link
Member

Thanks for your interest in palantir/blueprint, @maclockard! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@@ -96,9 +96,7 @@ export class EditableCell extends React.Component<IEditableCellProps, IEditableC

public componentWillReceiveProps(nextProps: IEditableCellProps) {
const { value } = nextProps;
if (value !== this.props.value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@maclockard i believe removing this check is wrong. it should instead by != null, just like EditableText does it. i'm going to close this PR and open my own with the correct change, and a refactor to the corresponding test.

thanks for suggesting this feature!

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.

3 participants