-
Notifications
You must be signed in to change notification settings - Fork 1
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
[#156101143][#156213326] Feature chat mentions upgrade #20
Conversation
armandgray
commented
Mar 23, 2018
- [#156101143] after deleting an @ mention moving the cursor to right after the “@” symbol
- [#156213326] sending on enter when clicked anywhere in the message
…fter the “@” symbol
@@ -371,6 +370,31 @@ export default class MentionsTextInput extends Component { | |||
} | |||
} | |||
|
|||
setSelection(start, end = start) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are start
and end
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the start and end of the selection!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eg: 1234 -> you could select the chars from 0, 4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh so these are indices, that wasn't too clear, but was able to gather from context. Might be useful to use flowtypes to say that they're numbers, e.g. start: number, end: number=start
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe call it setSelectionFromIndices
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its called selection its a prop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the name is consistent with the name of the prop and the name of the passed param to onSelectionChange
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ie selection: {start, end}
is what RN has set as the naming convention
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gotcha
src/MentionsTextInput.js
Outdated
@@ -526,12 +569,17 @@ export default class MentionsTextInput extends Component { | |||
return !text1 && text2.length > 1 || text1.length < text2.length - 1; | |||
} | |||
|
|||
hasNewLineChar(text) { | |||
return text && text.length > 0 && text[text.length - 1] == '\n' | |||
|| text.indexOf('\n') !== -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two checks at the end seem redundant, it looks like you could get rid of the text[text.length - 1] == '\n'
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍