Skip to content

Conversation

@chiragsalian
Copy link
Contributor

@chiragsalian chiragsalian commented Sep 10, 2020

Fixes

#457

Tests

  1. Write up a message. Click in the middle of the message. Type out something and confirm the cursor doesn't automatically move to the end.
  2. Type out a really long message such that the input box handles multiple lines. Refresh and confirm the message remains and its multiline
  3. Type out a message really fast and click send. Confirm it gets sent just fine and the input box is empty.
  4. On the mobile app type out a message and confirm you can type quickly.
  5. On the mobile app center your cursor in the middle of the text and type out and confirm its working normally.

@chiragsalian chiragsalian changed the title [WIP] Testing some stuff Fixing comment edits when cursor is in the middle Sep 11, 2020
@chiragsalian chiragsalian self-assigned this Sep 11, 2020
@chiragsalian
Copy link
Contributor Author

Ready for review.

@chiragsalian chiragsalian changed the title Fixing comment edits when cursor is in the middle [WIP] Fixing comment edits when cursor is in the middle Sep 11, 2020
@chiragsalian
Copy link
Contributor Author

chiragsalian commented Sep 11, 2020

Updated code and tested on web and ios. Ready for review 👍

@chiragsalian chiragsalian changed the title [WIP] Fixing comment edits when cursor is in the middle Fixing comment edits when cursor is in the middle Sep 11, 2020
AndrewGable
AndrewGable previously approved these changes Sep 11, 2020
Copy link
Contributor

@AndrewGable AndrewGable left a comment

Choose a reason for hiding this comment

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

Looks good and tests well!

marcaaron
marcaaron previously approved these changes Sep 12, 2020
Copy link
Contributor

@marcaaron marcaaron left a comment

Choose a reason for hiding this comment

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

LGTM!

// If it does let's update this.comment so that it matches the defaultValue that we show in textInput.
if (this.props.comment && prevProps.comment === '' && prevProps.comment !== this.props.comment) {
this.comment = this.props.comment;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB, it's bugging me a little that we can't do this in the constructor but not much we can do right now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it's a little annoying but its the best i could think off such that we could still use debounce and have this.comment always be up to date.

@chiragsalian chiragsalian dismissed stale reviews from marcaaron and AndrewGable via ed8551f September 12, 2020 00:18
@chiragsalian
Copy link
Contributor Author

Updated to address recent review comment.

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.

4 participants