Add Video Call Chat Links #1878

Luke9389 · 2021-03-18T23:06:55Z

Details

This change introduces a new menu for video chats. Currently, the implementation just brings up a new video chat (zoom or google meet) and then we expect the user to share the link in the chat.

Possible Improvement

It would be great to have the generated meeting link show up in the chat. This seems possible with Zoom because they have an API, but unfortunately Google Meet doesn't. I'm a bit hesitant to implement it for one but not the other, and since it was not part of the initial issue, I'm going to leave that out for now.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/156264

Tests / QA Steps

The test steps here are pretty straight-forward.

Go to a chat with any other person (test account or not)
Click the little phone icon at the top right
Select either zoom or google meet (they'll both want you to sign in first, so the first time you click either of them, you'll need to sign into their platform)
Verify that you're brought to a new video chat.

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

…into luke-add-video-chat-links

assets/images/google-meet.svg

Luke9389 · 2021-03-22T22:04:05Z

src/components/Icon/index.js

-    src: PropTypes.oneOf(_.values(Expensicons)).isRequired,
+    src: PropTypes.oneOfType([
+        PropTypes.oneOf(_.values(Expensicons)),
+        PropTypes.func,


This is because I was reluctant to add the Zoom and Meet logos to a file called Expensicons 😄. Willing to do so, but this was my preferred method.

I would say just do it! We could just as easily have called it IconLibrary or something, but Expensicons is just a fun name. No need to let that affect the implementation imo.

Good call just importing the full-color svg directly where you used it. 👍🏼
But I think that means that this change is unnecessary.

Oh, I see why you need this. My mistake 👍

Sorry I'm not seeing it where is this a func ?

Are the svg imported as functions? 🤔

They are imported as functional components

Huh weird OK

So then... validating that it is

PropTypes.oneOf(_.values(Expensicons))

serves no purpose at all ?

Ah yeah good point. I think that is redundant now since any function will also pass this type check

src/pages/home/HeaderView.js

src/components/VideoChatMenu.js

roryabraham

I know this is WIP, just some early feedback.

assets/images/Zoom - Blue.svg

src/components/VideoChatMenu.js

src/pages/home/HeaderView.js

…into luke-add-video-chat-links

Luke9389 · 2021-03-31T22:40:15Z

For some reason I'm not able to reply to this comment so I'l do that here.

This is a good point. Since Expensicons is a subset of func, the Expensicons check isn't really doing anything. I added the func because I don't think the Zoom and Google Meet Icons should go in the Expensicons file. Perhaps we could make another file for non-expensify icons that we could then use here. Something like ThirdPartyIcons or OtherIcons and then the type check could be

PropTypes.oneOfType([
        PropTypes.oneOf(_.values(Expensicons)),
        PropTypes.oneOf(_.values(ThirdPartyIcons)),
        ...

What do you think @marcaaron?

marcaaron · 2021-03-31T23:06:57Z

I don't have a strong preference tbh, but I would lean towards just keeping the propTypes simple.

Luke9389 · 2021-03-31T23:10:15Z

Ok cool, then I think I'll just go for func and take the Expensicons check out 👍

Luke9389 · 2021-03-31T23:16:33Z

Updated to address most recent review (I also changed the animation on close to go up, rather than down, which isn't reflected in the commit names)

isagoico · 2021-04-01T22:43:12Z

Title:
Desktop - Chat - Video call modal only visible in full screen mode

Expected Result

Video call modal should always be visible

Actual Result

Video call modal only visible in full screen mode

Action Performed

Install desktop build
Login with an existing account
Open chat and select video call
Resize app

Platform

Issue confirmed in:

Desktop: ✔️

Build: 1.0.11-0

Notes/Images/Videos

Video

Luke9389 · 2021-04-01T22:53:55Z

@isagoico
I think I may know the cause of this problem.... This modal only determines its position once (on page load). So if you change the screen size at all without reloading the page, it will stay at the position it would have been in when it loaded.

Could I ask for a retest with a reload in between changing from fullscreen to normal?

If this does end up being the case, it's something we're aware of and plan to fix in a later PR.

isagoico · 2021-04-01T23:24:15Z

While exploring if this issue was also reproducible in web I found this behaviour:

Grabando.177.mp4

I asked a tester with MacOS to check if this was reproducible in the desktop app and indeed it was the same.
Seems like it's the behaviour Luke mentioned above. Will check off the PR since it was a pass and this issue it's a known behaviour.

Luke9389 · 2021-04-01T23:30:00Z

Thank you @isagoico 🙇

Luke9389 · 2021-04-01T23:30:36Z

Also I hope you're enjoying Valheim (and maybe Skatebird as well?) 😄

isagoico · 2021-04-02T01:26:12Z

@Luke9389 QQ, In mobile app, is it expected that we are redirected to this screen instead of the new meeting. In web we are redirected to the already created meeting in google meets.

Luke9389 · 2021-04-02T15:33:39Z

I think this will depend on whether you have the google meet app or not. Functionally, it's the same because our current solution still relies on the user to copy paste the invitation link into the chat anyway. I think this is fine.

shawnborton · 2021-04-05T16:50:11Z

Sorry for the late feedback, but for desktop - is it possible to animate the call menu's starting launch point from the call icon itself and not from the very top of the application?

roryabraham · 2021-04-05T16:57:06Z

is it possible to animate the call menu's starting launch point from the call icon itself and not from the very top of the application?

It's possible, yes. But I do think it might be more complexity than is worth it, because right now we're just using a string to define a pre-made animation, but I think to do what you've suggested would require a custom animation.

shawnborton · 2021-04-05T17:11:24Z

Sounds good. I can see where we will want this in other places too, but maybe we can wait until we have more use cases for them and then we can create a contributor issue for it.

Luke9389 · 2021-04-05T18:08:39Z

Shawn, keep in mind that if you go from full screen to windowed, the position of this modal will be off. It's possible the effect is more dramatic on your end for that reason.

shawnborton · 2021-04-05T18:40:06Z

I didn't change the size of my screen. I am just observing that the option menu launches from the top edge of the screen, which is weird, because my mouse clicked on an icon that was not at the top edge of the screen.

marcaaron · 2021-04-05T21:01:29Z

Can't we just use a simple fade in/out animation like we do when we right click to reveal the other context menu?

shawnborton · 2021-04-05T21:05:33Z

Oh, I love that idea!

Luke9389 · 2021-04-06T21:12:17Z

Sounds good to me! I'll whip up a PR for this

initial commit

2eda7b9

Luke9389 self-assigned this Mar 18, 2021

Luke9389 requested a review from a team as a code owner March 18, 2021 23:06

botify requested review from Gonals and removed request for a team March 18, 2021 23:07

Luke Donahue added 15 commits March 19, 2021 10:18

turned reportscreeen into a class component

d495521

added a button

a1c657e

Merge branch 'master' of https://github.com/Expensify/Expensify.cash …

aecd46e

…into luke-add-video-chat-links

cleaned up

917ca42

scaffold popover in report screen

e506b41

clean up

d1e4a8a

Merge branch 'master' of https://github.com/Expensify/Expensify.cash …

19e1500

…into luke-add-video-chat-links

Merge branch 'master' of https://github.com/Expensify/Expensify.cash …

2800fe5

…into luke-add-video-chat-links

set up a component for the menu

2266752

Merge branch 'master' of https://github.com/Expensify/Expensify.cash …

0bc93f7

…into luke-add-video-chat-links

added zoom and meet links and svgs

3e919b4

made the menu

2a783c9

loosened Icon prop conditions to allow for non-expensify content

0ec1237

whoops

48d4b1d

added style for video chat menu

a75f4df

Luke9389 commented Mar 22, 2021

View reviewed changes

assets/images/google-meet.svg Show resolved Hide resolved

Luke9389 commented Mar 22, 2021

View reviewed changes

src/pages/home/HeaderView.js Outdated Show resolved Hide resolved

took out custom style

1707c2f

Luke9389 commented Mar 22, 2021

View reviewed changes

src/components/VideoChatMenu.js Outdated Show resolved Hide resolved

style - keeping class comp for now

ec774a4

roryabraham reviewed Mar 23, 2021

View reviewed changes

assets/images/Zoom - Blue.svg Outdated Show resolved Hide resolved

src/components/VideoChatMenu.js Outdated Show resolved Hide resolved

src/pages/home/HeaderView.js Outdated Show resolved Hide resolved

Luke Donahue added 3 commits March 24, 2021 11:47

Merge branch 'master' of https://github.com/Expensify/Expensify.cash …

04d9b5c

…into luke-add-video-chat-links

updated zoom icon

692e9ad

refactored and included phone.svg

ebbdbd8

Luke Donahue added 3 commits March 31, 2021 16:13

style

e9c932a

added menuItems array to constructor

d994dd7

changed Icon propTypes

f2f82b9

marcaaron mentioned this pull request Mar 31, 2021

Add STYLING.md to clarify styling best practices #2185

Merged

marcaaron self-requested a review April 1, 2021 00:19

marcaaron approved these changes Apr 1, 2021

View reviewed changes

marcaaron merged commit c6e2eff into master Apr 1, 2021

marcaaron deleted the luke-add-video-chat-links branch April 1, 2021 00:24

OSBotify mentioned this pull request Apr 1, 2021

Deploy Checklist: Expensify.cash 2021-03-19 #1926

Closed

85 tasks

Luke9389 mentioned this pull request Apr 26, 2021

Call Options menu doesn't dynamically scale/position when the window size is changed #2517

Closed

5 tasks

aimane-chnaif mentioned this pull request May 8, 2023

[HOLD for payment 2023-04-26] [$1000] Android - Double click to close phone popup reopens the popup #16582

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Video Call Chat Links #1878

Add Video Call Chat Links #1878

Luke9389 commented Mar 18, 2021 •

edited

Loading

Luke9389 Mar 22, 2021

roryabraham Mar 23, 2021

roryabraham Mar 25, 2021

roryabraham Mar 25, 2021

marcaaron Mar 31, 2021

marcaaron Mar 31, 2021

roryabraham Mar 31, 2021

marcaaron Mar 31, 2021

marcaaron Mar 31, 2021

roryabraham Mar 31, 2021

roryabraham left a comment

Luke9389 commented Mar 31, 2021

marcaaron commented Mar 31, 2021

Luke9389 commented Mar 31, 2021

Luke9389 commented Mar 31, 2021

isagoico commented Apr 1, 2021 •

edited

Loading

Luke9389 commented Apr 1, 2021

isagoico commented Apr 1, 2021

Luke9389 commented Apr 1, 2021

Luke9389 commented Apr 1, 2021

isagoico commented Apr 2, 2021

Luke9389 commented Apr 2, 2021

shawnborton commented Apr 5, 2021

roryabraham commented Apr 5, 2021

shawnborton commented Apr 5, 2021

Luke9389 commented Apr 5, 2021

shawnborton commented Apr 5, 2021

marcaaron commented Apr 5, 2021

shawnborton commented Apr 5, 2021

Luke9389 commented Apr 6, 2021

Add Video Call Chat Links #1878

Add Video Call Chat Links #1878

Conversation

Luke9389 commented Mar 18, 2021 • edited Loading

Details

Possible Improvement

Fixed Issues

Tests / QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

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

roryabraham left a comment

Choose a reason for hiding this comment

Luke9389 commented Mar 31, 2021

marcaaron commented Mar 31, 2021

Luke9389 commented Mar 31, 2021

Luke9389 commented Mar 31, 2021

isagoico commented Apr 1, 2021 • edited Loading

Expected Result

Actual Result

Action Performed

Platform

Luke9389 commented Apr 1, 2021

isagoico commented Apr 1, 2021

Luke9389 commented Apr 1, 2021

Luke9389 commented Apr 1, 2021

isagoico commented Apr 2, 2021

Luke9389 commented Apr 2, 2021

shawnborton commented Apr 5, 2021

roryabraham commented Apr 5, 2021

shawnborton commented Apr 5, 2021

Luke9389 commented Apr 5, 2021

shawnborton commented Apr 5, 2021

marcaaron commented Apr 5, 2021

shawnborton commented Apr 5, 2021

Luke9389 commented Apr 6, 2021

Luke9389 commented Mar 18, 2021 •

edited

Loading

isagoico commented Apr 1, 2021 •

edited

Loading