[HOLD for payment 2022-05-20][$250] One of the popup modal options is highlighted by default when shown on click - reported by @Santhosh-Sellavel

[mvtglobally]

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. Open New Dot Web App or Desktop App
2. Go to any user chat click on ➕ ,
3. Go to Settings -> Profile -> Click on camera icon
4. Go to Settings -> Workspace -> General Settings -> Click on camera icon to change picture

Expected Result:

Popup Options shown should highlight on hover & click over the option

Actual Result:

One of the option is highlighted by default

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.1.41-3
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2022-03-06.at.11.40.22.PM.mov

Screen.Recording.2022-03-06.at.11.52.24.PM.mov

Expensify/Expensify Issue URL:
Issue reported by: @Santhosh-Sellavel
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1646591662321989

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @JmillsExpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[JmillsExpensify]

Hmm, that's odd. Agree that we shouldn't be highlighting any of the options in the dropdown by default.

[MelvinBot]

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

[MelvinBot]

Current assignee @JmillsExpensify is eligible for the External assigner, not assigning anyone new.

[roryabraham]

This can be external but is low priority so I'm making it a monthly.

[JmillsExpensify]

Thanks! Went ahead and created a job on Upwork in the meantime: https://www.upwork.com/jobs/~01a9940594deaadefa

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

[MelvinBot]

📣 @Santhosh-Sellavel You have been assigned to this job by @melvin-bot[bot]!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

Current assignee @roryabraham is eligible for the Exported assigner, not assigning anyone new.

[metehanozyurt]

Proposal

Cause

We have used "fadeInUp" and "fadeInDown" in several component, as animation prop of PopoverMenu. When popover opens, it comes from under mouse cursor. This causes random items look like hovered.
Please see slo-mo video:

20220324_215040.mp4

Solution

We can delete animationIn props from following files:

These components uses "fadeInUp" animation with PopoverMenu:

• App/src/components/ButtonWithMenu.js

  Line 85 in 6803d00

  animationIn="fadeInUp"

• App/src/pages/home/report/ReportActionCompose.js

  Line 449 in eb3b18e

  animationIn="fadeInUp"

  ,

These components uses "fadeInDown" animation with PopoverMenu:

• App/src/components/AvatarWithImagePicker.js

  Line 223 in 2b7d6d0

  animationIn="fadeInDown"

• App/src/components/VideoChatButtonAndMenu.js

  Line 135 in 0d1ec3d

  animationIn="fadeInDown"

• App/src/components/ThreeDotsMenu/index.js

  Line 97 in ff3625f

  animationIn="fadeInDown"

With this way, animationIn prop will fallback to default which is "fadeIn".
It will not effect the componenent visually, since animationInTiming is 1ms.

[Santhosh-Sellavel]

@roryabraham
The proposal solves the issue.

But few things to ensure.
Did we intentionally make animationInTiming to 1ms, I think is this is not expected behavior.
Can we check this one with the UX/design team first?

[roryabraham]

Did we intentionally make animationInTiming to 1ms, I think is this is not expected behavior.

I'm not sure ... check the git blame?

[Santhosh-Sellavel]

@roryabraham yes it was done intentionally here #6121 for web/desktop

[Santhosh-Sellavel]

@metehanozyurt Thanks for the props. Looks good.
Since we would still need the props on mobile platforms, can you submit an updated proposal to handle it without affecting mobile platforms?

[JmillsExpensify]

@Santhosh-Sellavel Checking in on this issue for further feedback on the above proposal?

[Santhosh-Sellavel]

Sorry for the delay I will check one ASAP today or by tomorrow.

[Santhosh-Sellavel]

@roryabraham,
Since we have decided to disable popover menu animation across web/desktop platforms here.

1. If we're are going to keep it that way then,

@metehanozyurt proposal looks good to me!

We don't need that function animateInImplementationForWeb, but instead let's just add the prop animationIn: 'fadeIn' straight away here

App/src/components/PopoverMenu/index.js

Line 23 in 3a58a6e

return <BasePopoverMenu {...props} onItemSelected={selectItem} />;

2. But if we are not sure about this,

then we could go with this proposal, instead removing the props let's add conditions based to differentiate a platforms and pass correct props.

Waiting for your thoughts. I feel like checking with the design team too.

[roryabraham]

I think the design is intentional so @metehanozyurt's proposal LGTM as well.

[melvin-bot]

📣 @metehanozyurt You have been assigned to this job by @roryabraham!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[Santhosh-Sellavel]

@roryabraham
Do you agree with this suggested change?

We don't need that function animateInImplementationForWeb, but instead let's just add the prop animationIn: 'fadeIn'straight away here

[roryabraham]

Yes, I agree

[Santhosh-Sellavel]

@metehanozyurt Any update?

[metehanozyurt]

Thank you @Santhosh-Sellavel , @roryabraham. Based on your valuable comments, if you allow, I am thinking of making a change as follows.

I will do my PR within 1 hour.

    // eslint-disable-next-line react/jsx-props-no-spreading
    return <BasePopoverMenu {...props} animationIn="fadeIn" onItemSelected={selectItem} />;

[Santhosh-Sellavel]

Pass fadeOut for animationOut @metehanozyurt, thanks!

[Santhosh-Sellavel]

PR is up review in progress

[JmillsExpensify]

Recent updates in the linked PR. Also heads up @roryabraham you have a question from Santhosh.

[JmillsExpensify]

Awaiting one more review in the linked PR.

[JmillsExpensify]

PR made it to staging yesterday.

[JmillsExpensify]

Still a couple more days to work through the regression period.

[metehanozyurt]

@JmillsExpensify I couldn't apply on Upwork it says "This job is no longer available.".
Would you be able to help with this? Thank you.

[JmillsExpensify]

Ok cool. I'm jumping in to close out all the payments and contracts, so let me figure that out for you while I round this out.

[JmillsExpensify]

@metehanozyurt and @Santhosh-Sellavel I've created a new job since the last one expired. Please apply here and I'll process your payment today: https://www.upwork.com/jobs/~01d530f583e9a9fdbf. Also @Santhosh-Sellavel I'll add the additional payment for reporting in your case. Thanks!

[Santhosh-Sellavel]

@JmillsExpensify Applied for C+, please add a milestone reporting bonus thanks!

[metehanozyurt]

Thank you @JmillsExpensify .

[JmillsExpensify]

Alright, I think we should be set here. Thanks everyone!