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

Add message length warning for sending messages to android clients #1200

Merged
merged 3 commits into from Jun 7, 2017

Conversation

Projects
None yet
3 participants
@ikarulus
Copy link
Contributor

ikarulus commented May 26, 2017

Rel: #285 #1049

Contributor checklist

  • My contribution is fully baked and ready to be merged as is
  • My changes are rebased on the latest master branch
  • My commits are in nice logical chunks
  • I have followed the best practices in my commit messages
  • I have made the choice whether I want the BitHub reward or not by omitting or adding the word FREEBIE in my Git commit messages
  • I have tested my contribution on these platforms:
  • Chromium 58.0.3029.81 on Arch Linux
  • My changes pass all the local tests 100%
  • I have considered whether my changes need additional tests, and in the case they do, I have written them

Description

@@ -529,6 +537,7 @@
return this.$('.bottom-bar form').submit();
}
this.toggleMicrophone();
this.toggleLenghtWarning();

This comment has been minimized.

@haffenloher

haffenloher May 31, 2017

Contributor

typo in the method name ("Lenght")

@scottnonnenberg

This comment has been minimized.

Copy link
Contributor

scottnonnenberg commented Jun 1, 2017

Tried this out, and I've got two concerns:

  1. The warning blows away the microphone icon (for recording voice notes) when it appears. How might we make those coexist?
  2. It shows even if the message is very short - if you add a file. What's the reasoning behind that?
@ikarulus

This comment has been minimized.

Copy link
Contributor

ikarulus commented Jun 2, 2017

  1. This happens anyway, always when I type the first letter.
  2. I was a distracted copycat concerning one function by reusing the one above. See commit. :)

@scottnonnenberg scottnonnenberg merged commit 1021f83 into signalapp:master Jun 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment