-
-
Notifications
You must be signed in to change notification settings - Fork 242
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
Multiline message #422
Multiline message #422
Conversation
I hope to patch my version and let people test it the next days. |
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.
As always, javascript is not my primary language. So consider this just comments.
@@ -51,7 +51,7 @@ | |||
</div> | |||
</div> | |||
<div class="jsxc_transfer jsxc_otr jsxc_disabled" /> | |||
<input type="text" class="jsxc_textinput" data-i18n="[placeholder]Message" /> | |||
<textarea class="jsxc_textinput" data-i18n="[placeholder]Message"></textarea> |
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.
Why a full </textarea>
instead of just a … />
?
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.
Textarea is not allowed to be self closing, because it can contain text. Probably jquery would take care of this, but I am used to it.
$(this).height($(this).data('originalHeight') * 1.5); | ||
} | ||
} | ||
|
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.
Is this a good place for a helper function? In between two win.find()
setup calls?
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.
This is a subject for debate 😆, but it is absolute valid javascript. As you can see at the top of the outer function, the complete function needs to be rewritten. The reason why I defined this helper function at this place was, that it is not used elsewhere in the code and therefore it is easier to read.
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.
Okay. Maybe this would be a good start to slowly rewrite it now?
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.
4.0 will transfer jsxc to typescript, so I would like to wait after 3.1 with bigger changes 😉 .
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.
Yeah, let's wait with some fun for 3.1.
For testing I applied the changes to 3.0.1, I hope the changes do not require any intermediate updates? In testing, I noticed one interesting behaviour on Chromium 55.0.2883.75: Also: |
Nope.
That is not the intended behavior.
Yes this is intended. I is similar to the behavior of mobil chat clients like signal. If you need more space you can enlarge the complete chat window, so that the textarea has more width. One thing which I didn't check it the behavior on small devices. Thank you for testing this new feature. |
Okay,
Just as a note: The mobile app Conversations increases the input area line by line, as you type. It increases it to the point, where your old chat is completely hidden and all your screen space is used for the input area.
Thanks for working on this! My users are already happier with this. |
What do you think, can we merge it? |
Yes, go ahead. |
@ChristianTacke @thfree please test and verify if possible
Line breaks are possible with shift+enter.