Skip to content
This repository has been archived by the owner on Dec 23, 2022. It is now read-only.

added onBeforeRequestAdd #146

Merged
merged 4 commits into from
Oct 11, 2017

Conversation

kalimantos
Copy link
Contributor

solves #145

  • added onBeforeRequestAdd(chip): Boolean
  • edited story for controlled input

Copy link
Member

@leMaik leMaik left a comment

Choose a reason for hiding this comment

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

I think this could be even simpler. Why do we need to change any state for this? 🤔

@@ -253,7 +253,7 @@ class ChipInput extends React.Component {
}

handleKeyDown = (event) => {
this.setState({keyPressed: false})
this.setState({keyPressed: false, preventChipCreation: false})
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this state and handle the keyDown event? Only checking what the function says in handleAddChip should be sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's all because of the this.clearInput() inside handleKeyUp
i need some more state to know when to prevent also the handleKeyUp

@@ -304,6 +304,7 @@ class ChipInput extends React.Component {
}

handleKeyUp = (event) => {
if (this.state.preventChipCreation) return
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is needed to avoid input to be cleared, i tried removing it but this is the result:
bug-clear-input

src/ChipInput.js Outdated
@@ -324,6 +325,9 @@ class ChipInput extends React.Component {
}

handleAddChip (chip) {
if (this.props.onBeforeRequestAdd && !this.props.onBeforeRequestAdd(chip)) {
return this.setState({ preventChipCreation })
Copy link
Member

Choose a reason for hiding this comment

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

A simple return should be sufficient here. Also, where does the preventChipCreation variable come from here?

@leMaik
Copy link
Member

leMaik commented Sep 28, 2017

Looks fine to me now, @kalimantos. Is there anything you want to do on this before I merge the PR?

@leMaik leMaik merged commit 50cd0f3 into TeamWertarbyte:master Oct 11, 2017
@jriuamb
Copy link

jriuamb commented Oct 11, 2017

When the first chip is verified, the content of input it continues clearing

@leMaik
Copy link
Member

leMaik commented Oct 11, 2017

@jriuamb I can't reproduce this. 😮

@jriuamb
Copy link

jriuamb commented Oct 13, 2017

I am new to github. I'm trying to understand how it works. I downloaded the master branch version 0.18.1. I tried the Controlled Input form StoryBook.
I have modified ControlledChipInput.js from stories folder, by setting the initial value of state with an empty array in the constructor. When you enter a value that onBeforeRequestAdd returns false, it does not clear the input.

class ControlledChipInput extends React.Component {
constructor (props) {
super(props)
this.state = {
chips: []
}
}
...

Also happens that the console shows react errors.

image

@leMaik
Copy link
Member

leMaik commented Oct 14, 2017

@jriuamb Thanks for the reproduction example! I'll look into this soon.

@kalimantos kalimantos deleted the added-on-before-add branch November 9, 2017 08:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants