[New-feature] Messages deletion - Read/Unread by mcallegari10 · Pull Request #168 · Wolox/react-chat-widget

mcallegari10 · 2020-04-08T14:37:02Z

New features

Add messages deletion fetaure as on feat: delete function with count and start id params #137 migrated to TypeScript
Add onTextInputChange new prop as on Add onchange event #79 migrated to TypeScript
Add accesibility props as on a11y: added ARIA live, chatId and labels for launcher button #120
Mark messages as read/unread
Change badge feature to handle automatically new messages received

Fixes

Fix addLinkSnippet type issue
Change prod configuration to export only the widget, not a minified version of React/ReactDOM

Other

Bump version to mark the 3.0 beta release
Remove the extension on styles files, change the .scss files to modules and change variables file to a folder

- Add onTextInputChange new prop - Fix addLinkSnippet typing issue

- mark messages as read/unread - change badge component to handle automatically new messages received - change prod configuration to export only the widget - bump version to mark the 3.0 beta

LucasZibell · 2020-04-08T14:44:27Z

dev/App.tsx

 import React, { Component } from 'react';

-import { Widget, addResponseMessage, setQuickButtons, toggleMsgLoader } from '../index';
+import { Widget, addResponseMessage, setQuickButtons, toggleMsgLoader, addLinkSnippet, toggleWidget } from '../index';


Are you using toggleWidget? I don't see it on this file 🤔

LucasZibell · 2020-04-08T14:48:40Z

src/components/Widget/components/Conversation/components/Messages/index.tsx

@@ -1,8 +1,10 @@
 import React, { useEffect, useRef } from 'react';
 import { useSelector, useDispatch } from 'react-redux';
+import { format } from 'date-fns';


Are you using a lot of functions from date-fns? If not you can just install date-fns/format and reduce the size a lot 😄

LucasZibell · 2020-04-08T14:51:06Z

src/components/Widget/components/Conversation/components/Messages/index.tsx

-  useEffect(() => scrollToBottom(messageRef.current), [messages]);
-
+  useEffect(() => {
+    console.log(showChat)


LucasZibell · 2020-04-08T14:52:24Z

src/components/Widget/components/Conversation/components/Messages/index.tsx

+    scrollToBottom(messageRef.current);
+    if (showChat && badgeCount) dispatch(markAllMessagesRead());
+    else dispatch(setBadgeCount(messages.filter((message) => message.unread).length));
+  }, [messages]);


Are you sure this dependencies are okey? I see that you are using lots of external values like showChat , badgeCount , dispatch, setBadgeCount

Yes, this array is not about dependencies but when the values change, the effect will run. So I just need the effect to run when the messages are changed. If I add the showChat value for example, the badge will never be reset and will show me the badge count whenever I toggle the widget.

I've added the badgeCount since I need it to run also there 😄

LucasZibell · 2020-04-08T14:54:12Z

src/components/Widget/components/Conversation/components/Messages/index.tsx

-  const typing = useSelector((state: GlobalState) => state.behavior.messageLoader);
  const dispatch = useDispatch();
+  const { messages, typing, showChat, badgeCount } = useSelector((state: GlobalState) => ({
+    messages: state.messages.messages,


If you add here: messages: state.messages.messages || [] You can avoid the validation below for the map. It will also know that messages will always be a list. What do you think?

I think it's not that necessary. The store initial state will ensure that it will always be at least an empty array. The validation on the JSX it's just for precaution 😄.

LucasZibell · 2020-04-08T14:57:13Z

src/components/Widget/components/Launcher/index.tsx

-    <button type="button" className={cn('rcw-launcher', { 'rcw-hide-sm': showChat })} onClick={toggle}>
-      <Badge badge={badge} />
+    <button type="button" className={cn('rcw-launcher', { 'rcw-hide-sm': showChat })} onClick={toggleChat}>
+      {!showChat && <Badge badge={badgeCount} />}


What do you think of showChat || <Badge badge={badgeCount} />?

Why change it or?

LucasZibell · 2020-04-08T15:07:26Z

src/store/actions/types.ts

 export type BehaviorActions = ToggleChat | ToggleInputDisabled | ToggleMsgLoader;

-export type MessagesActions = AddUserMessage | AddResponseMessage | AddLinkSnippet | RenderCustomComponent | DropMessages | HideAvatar;
+export type MessagesActions = AddUserMessage | AddResponseMessage | AddLinkSnippet | RenderCustomComponent


I have my doubts about this. What is the advantage of having one type MessagesActions that can have lots of interfaces. I think the | should be use when we have lots of items that go perfect in 2 or 3 groups of types so we can group them. Here i just see a group of lots of types that each of them represents only 1 action. I think this type MessagesActions should be removed and use the specific types for each action. If you have each specific type for each action the search will be easier

The advantage of having this type is to describe the dispatch for each reduce with its action type.

I think that it's not necessary to explicitly type each action creator, that could be refactored.

LucasZibell · 2020-04-08T15:07:33Z

src/store/actions/types.ts

+  type: typeof MARK_ALL_READ;
 }

 export type BehaviorActions = ToggleChat | ToggleInputDisabled | ToggleMsgLoader;


LucasZibell · 2020-04-08T15:13:41Z

src/store/reducers/messagesReducer.ts

+  [DELETE_MESSAGES]: (state: MessagesState, { count, id }) => {
+    if (id) return { ...state, messages: state.messages.filter(message => message.customId !== id) }
+    return { ...state, messages: state.messages.splice(state.messages.length - 1, count) }
+  },


[DELETE_MESSAGES]: (state: MessagesState, { count, id }) => ({ ...state, messages: id ? state.messages.filter(message => message.customId !== id) : state.messages.splice(state.messages.length - 1, count) } }),

- change date-fns import path - add dependency to useffect in Messages component - change messages reducer to ternary in delete messages handle

- add new functions to the main export - remove badge prop

- add align start to timestamp in response messages - fix production build to not add the devtools

pabloferro · 2020-04-14T13:56:43Z

src/components/Widget/components/Conversation/components/Messages/index.tsx

+    scrollToBottom(messageRef.current);
+    if (showChat && badgeCount) dispatch(markAllMessagesRead());
+    else dispatch(setBadgeCount(messages.filter((message) => message.unread).length));
+  }, [messages, badgeCount]);


Shouldn't showChat be a dependency here?

mcallegari10 added 2 commits April 5, 2020 20:09

- Add messages deletion

1e14526

- Add onTextInputChange new prop - Fix addLinkSnippet typing issue

New features

f66ea9d

- mark messages as read/unread - change badge component to handle automatically new messages received - change prod configuration to export only the widget - bump version to mark the 3.0 beta

mcallegari10 requested review from damfinkel and pabloferro April 8, 2020 14:37

mcallegari10 assigned pabloferro and damfinkel Apr 8, 2020

LucasZibell suggested changes Apr 8, 2020

View reviewed changes

mcallegari10 added 3 commits April 9, 2020 17:52

- add accesibility features according to #120

8d555a0

- change date-fns import path - add dependency to useffect in Messages component - change messages reducer to ternary in delete messages handle

- add missing id parameter for the dispatcher

7e17444

- add new functions to the main export - remove badge prop

- change action types return

841d3ce

- add align start to timestamp in response messages - fix production build to not add the devtools

LucasZibell approved these changes Apr 13, 2020

View reviewed changes

pabloferro reviewed Apr 14, 2020

View reviewed changes

pabloferro approved these changes Apr 14, 2020

View reviewed changes

add dependency on useEffect in Messages component

d1feb13

mcallegari10 merged commit 4705a23 into 3.0 Apr 14, 2020

mcallegari10 deleted the prs-features-refactor branch April 14, 2020 18:40

Conversation

mcallegari10 commented Apr 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New features

Fixes

Other

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mcallegari10 commented Apr 8, 2020 •

edited

Loading