Conversations UI - create chat window #421

olipyskoty · 2021-02-17T02:38:23Z

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

JIRA link(s):

VIDEO-3731

Description

created chat button using svg icon from figma designs
created close chat window button using svg icon from figma designs
created ChatProvider and useChatContext hook
created ToggleChatButton component and added to MenuBar component
created ChatWindow component
added ChatWindow component to Room component and included ui for chat window on web and mobile
created ChatWindowHeader component and added to ChatWindow component
created CloseChatWindowButton component and added to ChatWindowHeader component
created unit tests for Room, CloseChatWindowButton, ToggleChatButton components and useChatContext hook
gif below shows chat window opening and closing on web and mobile!
note: when gif gets to mobile example, there appears to be whitespace in the chat header. I tested this on my iphone and locally in Device Mode in the Chrome Devtools and confirm it only happens in the gif 🤷‍♀️

Burndown

Before review

Updated CHANGELOG.md if necessary
Added unit tests if necessary
Updated affected documentation
Verified locally with npm test
Manually sanity tested running locally
Included screenshot as PR comment (if needed)
Ready for review

Before merge

Got one or more +1s
Re-tested if necessary

…akeStyles() method

timmydoza · 2021-02-18T00:50:42Z

src/components/Buttons/CloseChatWindowButton/CloseChatWindowButton.tsx

+  };
+
+  return (
+    <div onClick={closeChatWindow}>


Nit: This function can just be inline. It's less code:

return ( <div onClick={() => setIsChatWindowOpen(false)}> <CloseChatWindowIcon /> </div> );

This change makes this component quite small, so I don't think it needs to be in its own file. What do you think about moving all of this (and tests) into the ChatWindowHeader component?

i agree 👍 i moved all CloseChatWindowButton code into the ChatWindowHeader component and deleted this folder from the Buttons directory

timmydoza · 2021-02-18T18:06:54Z

src/components/Buttons/CloseChatWindowButton/CloseChatWindowButton.tsx

+  };
+
+  return (
+    <div onClick={closeChatWindow}>


Looks like the close icon isn't perfectly centered. There seems to be a little extra vertical space in that div:

Seems that adding display: flex to the div fixes the issue. For a single css property, I think it's fine to just add it as a style prop:

<div style={{ display: 'flex' }}>

nice find. because i moved all of this code to the ChatWindowHeader component, I was able to add this css to the existing useStyles object and avoid using the style prop

timmydoza · 2021-02-18T18:12:42Z

src/icons/CloseChatWindowIcon.tsx

@@ -0,0 +1,20 @@
+import React from 'react';
+
+export default function CloseChatWindowIcon() {


I think this icon can be named a little more generically. It's just an "X", so I don't think it is specific to the ChatWindow component. What do you think about CloseIcon?

That makes more sense so that we can reuse it in the future if need be 👍

timmydoza · 2021-02-18T18:15:14Z

src/theme.ts

@@ -121,4 +125,6 @@ export default createMuiTheme({
  sidebarMobilePadding: 8,
  participantBorderWidth: 2,
  mobileTopBarHeight: 52,
+  chatWindowWidth: 320,
+  chatWindowHeaderHeight: 56,


I think it would be ok if this value is stored inside the ChatWindowHeader component itself. I think the theme object is a good place to put values that will be used in multiple spots. I think that the chatWindowHeaderHeight value is only going to be used by that component.

I think it's fine to keep chatWindowWidth in here though.

i will keep that in mind! removed and updated ChatWindowHeader

timmydoza · 2021-02-18T18:15:55Z

src/hooks/useChatContext/useChatContext.ts

+import { useContext } from 'react';
+import { ChatContext } from '../../components/ChatProvider';
+
+export default function useVideoContext() {


Should be useChatContext.

Suggested change

export default function useVideoContext() {

export default function useChatContext() {

timmydoza · 2021-02-18T18:31:01Z

src/components/ChatWindowHeader/ChatWindowHeader.tsx

+      justifyContent: 'space-between',
+      alignItems: 'center',
+      paddingLeft: '1em',
+      paddingRight: '1em',


Looks like the designs have a border on the bottom of this header.

Nit - equivalent would be: padding: 0 1em

made update to padding. i used boxShadow as opposed to borderBottom as i think this looks closer to the designs, but let me know what you think!

timmydoza · 2021-02-18T18:33:49Z

src/components/ChatWindowHeader/ChatWindowHeader.tsx

@@ -0,0 +1,36 @@
+import React from 'react';


When it comes to organizing component directories, I like to keep important components and components that may be used in many places at the top level of the component/ directory. Since this component is a minor component that will only be used by the ChatWindow component, I think it should be moved inside that component:

src/components/ChatWindow/ChatWindowHeader/ChatWindowHeader.tsx.

src/components/Buttons/CloseChatWindowButton/CloseChatWindowButton.test.tsx

timmydoza · 2021-02-18T18:39:39Z

src/components/Buttons/CloseChatWindowButton/CloseChatWindowButton.test.tsx

+describe('the CloseChatWindowButton component', () => {
+  it('should render correctly when chat window is open', () => {
+    const wrapper = shallow(<CloseChatWindowButton />);
+    expect(wrapper.containsMatchingElement(<CloseChatWindowIcon />)).toEqual(true);


I don't think this test is needed. When shallow rendering this component, I think the icon will always be there regardless of the value of isChatWindowOpen (which is actually undefined here because it isn't defined on line 14. I think this is ok because this value is not relevant to this test.)

timmydoza · 2021-02-18T18:44:24Z

src/components/Buttons/ToggleChatButton/ToggleChatButton.test.tsx

+  it('should call the correct toggle function when clicked', () => {
+    const wrapper = shallow(<ToggleChatButton />);
+    wrapper.find(Button).simulate('click');
+    expect(mockToggleChatWindow).toHaveBeenCalled();


I think here we can test that the button is correctly negating the value of isChatWindowOpen. Let's define isChatWindowOpen (say as false) on line 14, then here on line 26 we can use .toHaveBeenCalledWith(true).

…ponent

timmydoza

Nicely done!! 🎉

* VIDEO 3731 Conversations UI - create chat window (#421) * Video 4078 add flip camera button to mobile menu (#433) VIDEO 4078 Conversations - Add flip camera button to 'more' mobile menu * Video 3730 add conversation hooks (#438) * Add initial hooks for chat * Update hooks in chat provider * Add tests for conversationProvider component * Fix DeviceSelectionScreen tests * Update variable names * Revert proxy url in package.json * Fix linting issues * Make error handling more user friendly * Fix a test in src/state * Temporarily disable cypress tests * Undo prettier changes to config.yml * Update chatProvider test * Cleanup ChatProvider * Video 3732 Conversations UI - create message components (#441) * Add links to TextMessage component (#446) * Add links to TextMessage component * Update CSS in TextMessage * Update snapshot test * Fix issue in addLinks function * Use forEach loop in addLinks function * Add new server (#443) * Use new server with reused code * Add firebase auth to server * Convert server to typescript * Add tests for firebaseAuthMiddleware * Refactor server * Add more server tests * Add tests for CreateExpressHandler * Fix linter issue * Delete old server * Fix linter issues * Fix some server issues * Set up server build steps * Change request to match new server api * Fix token fetch test * Add server build step to circleci config * Enable cyress tests in CI * Undo prettier changes in config.yml * install plugin-rtc from npm * Fix firebaseAuth test * Add create_conversation:true to token requests * Upgrade plugin-rtc to 0.8.0 * Temorarily remove firebaseAuthMiddleware and recording rules route * Add ChatProvider back in * Update .env.example * Change build:server script * Add firebaseAuthMiddleware * Add comments to firebaseAuthMiddleware * Update useFirebaseAuth tests * Video 3728 Conversations UI - add input field and send button (#451) * Chat notification (#454) * Add notification to chat icon * Add tests for ToggleChatButton * Some cleanup * Fix linting issue * Video-4525 loading screen (#461) * Video 4454 Conversations UI - message list scrolling (#467) * Update MessageList component to change how messages are grouped (#469) * Update MessageList component to change how messages are grouped * Simplify MessageList code * Video 3888 file input for conversations (#458) * Add FileAttachmentIcon * Add File Input to ChatInput component * Add change handler to file input * Create MediaMessage component and add it to MessageList * Toggle ChatWindow visibility with CSS * Update messageList tests * Add function to format the file size * Update error handling in ChatInput component * Add tests for MediaMessage component * Update snapshot test * Add tests for ChatInput * Fix linting issue * Update filesize formatting * Update error language and tests * Reset file input value after file sent * Center icon in MediaMessage * Update Icon size * Fix breaking ChatInput test * Update snapshot * Use accept attribute on file input element * Update readme to add information about the chat feature and conversations api * Cross browser fixes (#472) * Update app.yaml for the new server * Add cache controls for static assets * Only hide Screenshare button on mobile devices * Auto focus on chat input when user opens chat window * Fix iOS bug with media messages * Fix issues in MessageListScrollContainer * Fix app.yaml * Fix infinite loop when there is a conversations error * Update tests for ChatInput * Update mediamessage test * Update menubar tests * Update messageList snapshot * Fix linting issue * Add comments to the server * Video 3727 - cypress testing (#473) * Video 4597 feature flag (#475) * Update plugin-rtc version * Add feature flag for Twilio Conversations * Add tests * Add info to .env.example * Update readme with info about twilio conversations (#476) * Update readme with info about twilio conversations * Update readme to address feedback * Release 0.3.0 (#479) * Bump version and update changelog * Add plugin-rtc upgrade instructions to the readme Co-authored-by: olipyskoty <77076398+olipyskoty@users.noreply.github.com> Co-authored-by: Sean Coleman <smcoleman27@gmail.com>

olipyskoty added 17 commits February 11, 2021 13:27

Add chat svg icon from designs

312623d

add chat button to menu bar

1efbdcb

add chat window to room and refactor materialui code to make use of m…

7f6e9a8

…akeStyles() method

add chat provider context to index.js

324106a

add chat window styles to theme

fdcf1e9

add chat button

26835c9

add chat window component

f8632cf

add chatProvider and useChatContext hook

8a4e3f7

add button for closing chat window

2c22fa5

add component for chat window header

cb06376

add chat window header to chat window component

b480fd7

correct formatting

4589592

change close chat window button to div to remove hover state

6a23206

correct styling for chat header

4f883d9

move flip camera button back to MenuBar for time being

a645876

remove FlipCameraButton from menubar

f6f2b5e

add unit tests for chat window and chat window button

5d2adba

olipyskoty requested a review from timmydoza February 17, 2021 02:38

timmydoza suggested changes Feb 18, 2021

View reviewed changes

olipyskoty added 7 commits February 18, 2021 17:51

remove chatWindowHeaderHeight from theme

178d409

delete CloseChat button folder to move code into ChatWindowHeader com…

87b8189

…ponent

rename icon to CloseChat icon

e380e46

make ToggleChatButton tests more specific

614f563

fix typo to usechatContext

4f5617a

remove snapshot tests from Room tests

b696528

move ChatWindowHeader component into ChatWindow component directory

1f844db

olipyskoty requested a review from timmydoza February 18, 2021 23:04

fix hover styling/css to match other menu buttons

d1773d7

timmydoza approved these changes Feb 19, 2021

View reviewed changes

olipyskoty merged commit aa2c8c1 into feature/conversations Feb 19, 2021

olipyskoty deleted the VIDEO-3731-conversations-ui-create-chat-window branch February 19, 2021 17:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conversations UI - create chat window #421

Conversations UI - create chat window #421

olipyskoty commented Feb 17, 2021 •

edited

Loading

timmydoza Feb 18, 2021

olipyskoty Feb 18, 2021

timmydoza Feb 18, 2021

olipyskoty Feb 18, 2021

timmydoza Feb 18, 2021

olipyskoty Feb 18, 2021

timmydoza Feb 18, 2021

olipyskoty Feb 18, 2021

timmydoza Feb 18, 2021

timmydoza Feb 18, 2021

timmydoza Feb 18, 2021

olipyskoty Feb 18, 2021

timmydoza Feb 18, 2021

timmydoza Feb 18, 2021

timmydoza Feb 18, 2021

timmydoza left a comment

		@@ -0,0 +1,20 @@
		import React from 'react';

		export default function CloseChatWindowIcon() {

	export default function useVideoContext() {
	export default function useChatContext() {

Conversations UI - create chat window #421

Conversations UI - create chat window #421

Conversation

olipyskoty commented Feb 17, 2021 • edited Loading

Pull Request Details

JIRA link(s):

Description

Burndown

Before review

Before merge

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timmydoza left a comment

Choose a reason for hiding this comment

olipyskoty commented Feb 17, 2021 •

edited

Loading