Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[#155997991] reloading trigger matrix when text is pasted in to inputbar #17

Merged
merged 2 commits into from
Mar 15, 2018

Conversation

armandgray
Copy link

No description provided.

@@ -535,6 +535,17 @@ export default class MentionsTextInput extends Component {
return;
}

const isTextDifferenceGreaterThanOne = !this.state.text && text.length > 1 || this.state.text.length < text.length - 1;
Copy link

@MattFoley MattFoley Mar 15, 2018

Choose a reason for hiding this comment

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

This would probably do well in it's own function too, right? It doesn't seem to have much effect on the rest of this function, and just needs the text value?

Copy link
Author

Choose a reason for hiding this comment

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

ah theres already a getTextDifference function. the difference is that this one checks the difference before the the state is changed

Choose a reason for hiding this comment

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

Yeah, but inline means you can't write a test for it, which I think is worthwhile, right?

Choose a reason for hiding this comment

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

I think it'd be worth putting this logic inside of its own function, too

Copy link
Author

Choose a reason for hiding this comment

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

👍

Copy link

@millibeckers millibeckers left a comment

Choose a reason for hiding this comment

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

👍

@leoleblanc
Copy link

I think the textDifferenceGreaterThanOne const should be its own function since it's doing computations that could potentially be useful later

Copy link

@leoleblanc leoleblanc left a comment

Choose a reason for hiding this comment

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

👍

@armandgray armandgray merged commit d610337 into master Mar 15, 2018
@armandgray armandgray deleted the feature-chat-mentions-upgrade branch March 15, 2018 17:00
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants