[HOLD for payment 2021-12-06] Web - Chat - Update popup animations on web/desktop to pop open instead of fade/slide

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Go to https://staging.new.expensify.comand login
2. Click the + icon in a Group DM text input and verify the popup menu

Expected Result:

The animation for the popup should open like in #6121 (comment). In addition to the example in the description, any other animation using fadeInUp/fadeOutDown should be updated on web/desktop.

Actual Result:

The animation for the popup slides and fades.

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web

Version Number: 1.1.1-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Bug5303247_Recording__158.mp4

Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @Jag96 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[Jag96]

@kavimuru I'm not sure I understand Popup in text input flies out and disappears for a moment. It looks like the issue is that when the pop-up menu comes up, one of the options can be auto highlighted, and then when moving the mouse the item is no longer highlighted (the fix would be to not highlight items while the animation is happening). Am I understanding that correctly?

[parasharrajat]

@Jag96 This is an extreme edge case and You can reproduce the same behaviour for the FAB menu.
Steps:

1. Click on the FAB.
2. When the menu is transitioning, continue clicking the FAB Button area.
3. Menu will toggle hide and show the state on every click.

[Jag96]

I'm still not seeing any bug in the description video other than the one in #6121 (comment), @kavimuru please can you clarify the Actual Result for me?

Menu will toggle hide and show the state on every click.

Is this the same issue that is being described in the description? @parasharrajat If it is the same issue, can you provide an Action Performed, Expected Result, and Actual Result for the FAB case as well?

[mvtglobally]

Issue reproducible during KI retests.

[mvtglobally]

@Jag96 I believe the issue minor. When you click on the popup it sort of slides overlapping with the content. We haven't seen it with other popups, so team decided to log it. See below 2 images

.

[Jag96]

@mvtglobally thanks for the context, it looks like this is being caused by the animation style for the PopoverMenu component, the same effect can be seen when clicking on an IOU detail and opening the option to pay:

Going to tag @Expensify/design here to confirm this is the behavior that we want, if not we can update the animation style and if so we can close this issue.

[parasharrajat]

Ok. Looking at the new comments, the issue reported here #6121 (comment) is different from this one.

@Jag96

Here is the video

output_file.mp4

[Jag96]

@parasharrajat good find! I think that can be filed separately then, feel free to post it in slack

[Jag96]

Adding design label to get input on #6121 (comment)

[MelvinBot]

Triggered auto assignment to @megankelso (Design), see these Stack Overflow questions for more details.

[MelvinBot]

@Jag96, @megankelso Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[megankelso]

hi, sorry for the delay. I believe this animation style is behaving as expected. But I see what you're saying and I feel like it could be cleaned up. What if instead of the fade and scroll up it just pops open?

such as in this example:

Screen.Recording.2021-11-09.at.1.44.19.PM.mov

[Jag96]

I agree that looks much cleaner. It looks like the fadeIn/fadeOut animation is a consistent default across the app (for example, try setting a profile picture), would we want to update the default to be no animation for all popovers like these?

[shawnborton]

I'm down to standardize on the in-place fade in/out for these kinds of things on desktop. For mobile, I still think we'd want to launch this menu (which actually becomes more like a modal) from the bottom though.

[Jag96]

Sounds good, do we want to leave the fadeIn/fadeOut for mobile as well? Or remove the fade on mobile and just have it slideInUp/slideOutDown rather than fadeInUp/fadeOutDown?

[shawnborton]

I think mobile is probably fine as-is, so I wouldn't change anything there. But curious if others agree too!

[megankelso]

I think mobile is fine but would be up for cleaning up the animation on web

[Jag96]

Sounds good! Updating the title and making external, thanks for the 👀

[parasharrajat]

For the timing props, do we need to update any platform-specific files to ensure that mobile still behaves as it currently does?

I think updating the timings for POPOVER in getModalStyles will be enough as it is not used on Mobile native.

Is there any way to just not have an animation set? If not, then can you post a video of what 80ms looks like? Ideally, we'd get it as close to #6121 (comment) as possible.

Docs do not say that it accepts null but I will check the code if it does I will pass that. Otherwise, we have to manage it with timings.

Question: do we not want to keep any animation at all?

[Jag96]

I think updating the timings for POPOVER in getModalStyles will be enough as it is not used on Mobile native.

👍

Docs do not say that it accepts null but I will check the code if it does I will pass that. Otherwise, we have to manage it with timings.

👍

Question: do we not want to keep any animation at all?

On mobile, we want to keep all animations as they are now, but on web/desktop we want to remove the animation.

@parasharrajat your proposal sounds good then, if you have any more questions feel free to ask otherwise feel free to start on a PR. @dylanexpensify let's hire @parasharrajat for this one!

[dylanexpensify]

On it!

[dylanexpensify]

@parasharrajat mind applying for the job?

[parasharrajat]

@dylanexpensify Just did. Sorry for not doing proactively.

[dylanexpensify]

haha no worries at all @parasharrajat! I've barely had coffee, so that reaction time was 🔥 :). Sent the offer!

[parasharrajat]

@Jag96 Do we need to remove animation from the FAB menu as well?

[Jag96]

I believe we want to keep that the same, @shawnborton @megankelso thoughts on the FAB menu animation?

[megankelso]

yes I think that one is ok as is!

[shawnborton]

The current FAB menu animation on desktop fades in and slides in from the left to right, which I think is super weird. I think we should update the FAB animation to match the other desktop styles we're proposing here.

[megankelso]

sorry I think I'm confused. to clarify, is this the animation you're referring to?

Screen.Recording.2021-11-17.at.8.05.13.AM.mov

[shawnborton]

Yup - I think that animation can be replaced with what we're recommending for other menu popups where the menu itself doesn't change horizontal or vertical position - it just fades in very quickly into place.

[parasharrajat]

hmm. just a flag away. Let me know your final thoughts.....

[parasharrajat]

so What should I do now finally for that? @shawnborton @megankelso

[shawnborton]

I think whatever desktop animation you are using for popups should be used for the FAB on desktop as well. Let's standardize on the same thing everywhere if possible.

[parasharrajat]

Ok. Updating the PR now.

[botify]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.16-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Remove animations from the Popover menus #6333

If no regressions arise, payment will be issued on 2021-12-06. 🎊

[parasharrajat]

Ping for

@dylanexpensify

[dylanexpensify]

On it now!

[dylanexpensify]

Payment sent, contract ended, job closed