[NEW] Message character limit feature #443

ogustavo-pereira · 2020-07-09T00:44:15Z

The main goal of this PR is to add functionality to add character limit for messages.

* adding new component ReminderCharacters on Footer * update function shouldComponentUpdate in Composer * adding config.settings.limitTextLength in screenProps

* added limitTextLength validation if null

renatobecker · 2020-07-15T13:18:37Z

Hey @oguhpereira thanks for contributing, It seems to be a very nice feature!

I have two points to discuss here before we review/merge your code:

We need the design implemented on the storybook first;
We need to discuss the design you implemented with our design team, so please share a new gif or a video demo of this feature with higher quality, it's hard to see it on the gif you shared.

I'll be waiting for some news.

Thanks.

ogustavo-pereira · 2020-07-21T00:52:37Z

Hi @renatobecker this is the original video.

I went to add the storybook but there seems to be a problem of compatibility of the preact with the version that is in the package.json

renatobecker · 2020-07-21T03:22:19Z

Hi @renatobecker this is the original video.

I went to add the storybook but there seems to be a problem of compatibility of the preact with the version that is in the package.json

I just watched your video, I noticed that even there is a message length limit, you're allowing the user to type new characters. why?

ogustavo-pereira · 2020-07-21T23:44:11Z

@renatobecker I based it on twitter's UX. I don't prevent the user from entering the message, just block the sending. I'm trying not to create a bad experience when I exceed the character limit. Therefore, even if the user types a message larger than expected, he can adapt to fit the limit.

renatobecker · 2020-07-21T23:51:49Z

@renatobecker I based it on twitter's UX. I don't prevent the user from entering the message, just block the sending. I'm trying not to create a bad experience when I exceed the character limit. Therefore, even if the user types a message larger than expected, he can adapt to fit the limit.

So, I see two points here:

if you are just blocking the sending, why do you display the message length limit?
do you think it's a good UX allowing the users to type hundred words and then tell them are not allowed to send the message because the limit has been reached?

Sry, this doesn't make sense to me.

ogustavo-pereira · 2020-07-22T01:14:50Z

if you are just blocking the sending, why do you display the message length limit?
do you think it's a good UX allowing the users to type hundred words and then tell them are not allowed to send the message because the limit has been reached?

For the user to see how many characters passed
In my opinion, it is a good UX to allow the user to type as much as he wants and then review and delete the excess until it fits the limit

renatobecker · 2020-07-22T11:59:15Z

if you are just blocking the sending, why do you display the message length limit?
do you think it's a good UX allowing the users to type hundred words and then tell them are not allowed to send the message because the limit has been reached?

For the user to see how many characters passed

In my opinion, it is a good UX to allow the user to type as much as he wants and then review and delete the excess until it fits the limit

I see.
I understand that for Twitter it may make sense, but here the users aren't blog posting, they're sending messages in a row, which means this isn't the UX our users are expecting for.
If you're willing to implement the feature according to our UI/UX, I will be able to review it.
In addition, there is a conflict in a file, it must be fixed too.

Thanks.

* Removing visual effects when exceeding message limit * Adding write block when passing the character limit value

ogustavo-pereira · 2020-07-22T23:03:43Z

If you're willing to implement the feature according to our UI/UX, I will be able to review it.

@renatobecker, I removed the changes commented above, added a lock to continue typing. Check on video

renatobecker · 2020-07-22T23:14:13Z

If you're willing to implement the feature according to our UI/UX, I will be able to review it.

@renatobecker, I removed the changes commented above, added a lock to continue typing. Check on video

Ohh that's awesome!
@oguhpereira what do you think about highlighting the character counter when the limit is reached?
Think about it..

* Creating class footer__remainder-alert * Changing to class footer__remainder-alert when textLength is equal character limit

ogustavo-pereira · 2020-07-22T23:46:11Z

@renatobecker When text reaches the limit it changes to $color-red. Check on video

renatobecker · 2020-07-27T21:02:09Z

src/components/App/index.js

@@ -182,6 +182,7 @@ export class App extends Component {
 		expanded,
 		alerts,
 		modal,
+		config,


You don't need this here, the StoreConsumer is propagating the config prop to the App components.

renatobecker · 2020-07-27T21:02:14Z

src/components/App/index.js

@@ -204,6 +205,7 @@ export class App extends Component {
 			onOpenWindow: this.handleOpenWindow,
 			onDismissAlert: this.handleDismissAlert,
 			dismissNotification: this.dismissNotification,
+			limitTextLength: config.settings.limitTextLength,


You don't need this here, the StoreConsumer is propagating the config prop to the App components.
You need to extract the prop value here:
https://github.com/RocketChat/Rocket.Chat.Livechat/blob/develop/src/routes/Chat/container.js#L338

And then pass it through the components.

renatobecker · 2020-07-27T23:39:44Z

src/components/Footer/styles.scss

@@ -41,6 +41,16 @@

 		@include pressable-button(2px);
 	}
+
+	&__remainder, &__remainder-alert  {


Suggested change

&__remainder, &__remainder-alert {

&__remainder {

margin-left: 10px;

min-width: 100px;

font-weight: bold;

&--highlight {

color: $color-red;

transition: color .2s;

}

}

renatobecker · 2020-07-27T23:41:12Z

src/components/Footer/index.js

@@ -43,3 +43,13 @@ export const FooterOptions = ({ children }) => (
 		{children}
 	</PopoverMenu>
 );
+
+
+export const ReminderCharacters = ({ className, style = {}, textLenght, limitTextLength }) => (


Suggested change

export const ReminderCharacters = ({ className, style = {}, textLenght, limitTextLength }) => (

export const CharCounter = ({ className, style = {}, textLength, limitTextLength }) => (

<span

className={createClassName(styles, 'footer__remainder', { highlight: textLength === limitTextLength }, [className])}

style={style}

>

{textLength} / {limitTextLength}

</span>

);

renatobecker · 2020-07-27T23:43:42Z

src/routes/Chat/component.js

@@ -2,7 +2,7 @@ import { Component } from 'preact';

 import { Composer, ComposerAction, ComposerActions } from '../../components/Composer';
 import { FilesDropTarget } from '../../components/FilesDropTarget';
-import { FooterOptions } from '../../components/Footer';
+import { FooterOptions, ReminderCharacters } from '../../components/Footer';


Suggested change

import { FooterOptions, ReminderCharacters } from '../../components/Footer';

import { FooterOptions, CharCounter } from '../../components/Footer';

renatobecker · 2020-07-27T23:45:25Z

src/routes/Chat/component.js

@@ -162,14 +172,15 @@ export default class Chat extends Component {
 									<ComposerAction onClick={this.handleUploadClick}>
 										<PlusIcon width={20} />
 									</ComposerAction>
-								)}
+								)}1


@oguhpereira what's the reason to add the 1 here?
I didn't get it, was it a mistake?
I noticed you left the 1 here after my review.

* Changing component name to CharCounter * Changing alert to hilight

renatobecker · 2020-07-28T17:00:50Z

src/routes/Chat/component.js

@@ -162,14 +172,15 @@ export default class Chat extends Component {
 									<ComposerAction onClick={this.handleUploadClick}>
 										<PlusIcon width={20} />
 									</ComposerAction>
-								)}
+								)}1


@oguhpereira what's the reason to add the 1 here?
I didn't get it, was it a mistake?
I noticed you left the 1 here after my review.

renatobecker · 2020-07-29T02:55:14Z

@oguhpereira the storybook is okay now, please pull the last changes from the develop branch and try again.
After that, you'll be able to add the new component to the storybook.

* Remove number 1 * Create storybook in stories

ogustavo-pereira · 2020-07-29T23:59:59Z

@renatobecker I removed the number 1, it is not part of the logic, I also added the storybook

[NEW] Setting a character limit

017a97d

* adding new component ReminderCharacters on Footer * update function shouldComponentUpdate in Composer * adding config.settings.limitTextLength in screenProps

dlassalle mentioned this pull request Jul 9, 2020

Feature Request: Configurable character limit setting for Livechat RocketChat/Rocket.Chat#17451

Closed

renatobecker self-assigned this Jul 9, 2020

dlassalle mentioned this pull request Jul 14, 2020

Design wireframes for Chat Bot WideChat/Rocket.Chat#307

Open

[FIX] Null limitTextLength validation

bafb085

* added limitTextLength validation if null

ogustavo-pereira mentioned this pull request Jul 14, 2020

[NEW][Omnichannel] Ability to set character message limit on Livechat widget RocketChat/Rocket.Chat#18261

Merged

7 tasks

ogustavo-pereira mentioned this pull request Jul 21, 2020

[FEAT] Configurable character limit on live chat WideChat/Rocket.Chat#360

Closed

7 tasks

[FEAT] Adding write lock

4c39979

* Removing visual effects when exceeding message limit * Adding write block when passing the character limit value

[FEAT] Highlighting character counter

becce00

* Creating class footer__remainder-alert * Changing to class footer__remainder-alert when textLength is equal character limit

renatobecker suggested changes Jul 27, 2020

View reviewed changes

[FIX] CharCounter

12657f5

* Changing component name to CharCounter * Changing alert to hilight

renatobecker suggested changes Jul 29, 2020

View reviewed changes

ogustavo-pereira added 2 commits July 29, 2020 20:01

Merge branch 'develop' into 416-configurable-character-limit

9b07d09

[FEAT] Storybook CharCounter

1c84f20

* Remove number 1 * Create storybook in stories

renatobecker changed the title ~~[NEW] Setting a character limit~~ [NEW] Message character limit feature Aug 3, 2020

renatobecker approved these changes Aug 3, 2020

View reviewed changes

renatobecker merged commit bd01e54 into RocketChat:develop Aug 3, 2020

dlassalle mentioned this pull request Aug 19, 2020

[Feature Request] Configurable character limit for user messages #416

Closed

ggazzo mentioned this pull request Aug 22, 2020

Release 1.7.0 #475

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NEW] Message character limit feature #443

[NEW] Message character limit feature #443

ogustavo-pereira commented Jul 9, 2020 •

edited by renatobecker

Loading

renatobecker commented Jul 15, 2020

ogustavo-pereira commented Jul 21, 2020

renatobecker commented Jul 21, 2020

ogustavo-pereira commented Jul 21, 2020

renatobecker commented Jul 21, 2020

ogustavo-pereira commented Jul 22, 2020

renatobecker commented Jul 22, 2020

ogustavo-pereira commented Jul 22, 2020

renatobecker commented Jul 22, 2020

ogustavo-pereira commented Jul 22, 2020

renatobecker Jul 27, 2020

renatobecker Jul 27, 2020

renatobecker Jul 27, 2020

renatobecker Jul 27, 2020

renatobecker Jul 27, 2020 •

edited

Loading

renatobecker Jul 27, 2020

renatobecker Jul 28, 2020

renatobecker Jul 28, 2020

renatobecker commented Jul 29, 2020 •

edited

Loading

ogustavo-pereira commented Jul 29, 2020

-	&__remainder, &__remainder-alert  {
+	&__remainder {
+		margin-left: 10px;
+		min-width: 100px;
+		font-weight: bold;
+		&--highlight {
+			color: $color-red;
+			transition: color .2s;
+		}
+	}

-export const ReminderCharacters = ({ className, style = {}, textLenght, limitTextLength }) => (
+export const CharCounter = ({ className, style = {}, textLength, limitTextLength }) => (
+	<span
+		className={createClassName(styles, 'footer__remainder', { highlight: textLength === limitTextLength }, [className])}
+		style={style}
+	>
+		{textLength} / {limitTextLength}
+	</span>
+);

	import { FooterOptions, ReminderCharacters } from '../../components/Footer';
	import { FooterOptions, CharCounter } from '../../components/Footer';

[NEW] Message character limit feature #443

[NEW] Message character limit feature #443

Conversation

ogustavo-pereira commented Jul 9, 2020 • edited by renatobecker Loading

renatobecker commented Jul 15, 2020

ogustavo-pereira commented Jul 21, 2020

renatobecker commented Jul 21, 2020

ogustavo-pereira commented Jul 21, 2020

renatobecker commented Jul 21, 2020

ogustavo-pereira commented Jul 22, 2020

renatobecker commented Jul 22, 2020

ogustavo-pereira commented Jul 22, 2020

renatobecker commented Jul 22, 2020

ogustavo-pereira commented Jul 22, 2020

renatobecker Jul 27, 2020

Choose a reason for hiding this comment

renatobecker Jul 27, 2020

Choose a reason for hiding this comment

renatobecker Jul 27, 2020

Choose a reason for hiding this comment

renatobecker Jul 27, 2020

Choose a reason for hiding this comment

renatobecker Jul 27, 2020 • edited Loading

Choose a reason for hiding this comment

renatobecker Jul 27, 2020

Choose a reason for hiding this comment

renatobecker Jul 28, 2020

Choose a reason for hiding this comment

renatobecker Jul 28, 2020

Choose a reason for hiding this comment

renatobecker commented Jul 29, 2020 • edited Loading

ogustavo-pereira commented Jul 29, 2020

ogustavo-pereira commented Jul 9, 2020 •

edited by renatobecker

Loading

renatobecker Jul 27, 2020 •

edited

Loading

renatobecker commented Jul 29, 2020 •

edited

Loading