[$500] Room - Selection list RHP does not show sliding animation while being dismissed

[izarutskaya]

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

---

Version Number: 1.4.55-0
Reproducible in staging?: Y
Reproducible in production?: N
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Go to staging.new.expensify.com.
2. Go to FAB > Start room.
3. Go to Room.
4. Click Workspace (or Who can post or Visibility).
5. Click outside the RHP.

Expected Result:

The RHP will show sliding animation while being dismissed.

Actual Result:

The RHP does not show sliding animation while being dismissed.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6420333_1710932931030.20240320_190525.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a45c6aecba3aa427
• Upwork Job ID: 1770474516290019328
• Last Price Increase: 2024-05-08

[melvin-bot]

Triggered auto assignment to @CortneyOfstad (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @rlinoz (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[izarutskaya]

@rlinoz I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

We think this issue might be related to the #vip-vsb.

[allgandalf]

This isn't a regression ig, #38015 actually fixed the issue that if we click outside of the selector we cllose the RHP, isn't that right @Krishna2323 ?

[rlinoz]

He did, but the videos in the Authors checklist there is an animation, and in the reviewers checklist there is not, so something happened along the way.

@Krishna2323 @hoangzinh

[rlinoz]

Not a deploy blocker though, so removing the label

[ghost]

I don't see this issue happening in my case on the latest main

Screen.Recording.2024-03-20.at.5.34.35.PM.mp4

[Krishna2323]

@rlinoz @GandalfGwaihir, thanks for the mention, checking...

[rlinoz]

@godofoutcasts94 you have to click outside the RHP.

[ghost]

Yes i did

[Pranav2612000]

Proposal

Please re-state the problem that we are trying to solve in this issue.

• Open up the RHP by using FAB > Start room
• Open up any Value picker ( Workspace, Who can post or Visibility )
• Click outside the RHP anywhere

We observe that the whole RHP closes. The expected result is that only the ValuePickerModal should
close and show the Room page again

What is the root cause of that problem?

In ValuePicker/index.tsx we are dismissing the Modal when pressed outside

App/src/components/ValuePicker/index.tsx

Line 59 in 3311062

onBackdropPress={Navigation.dismissModal}

This causes the whole modal to close

What changes do you think we should make in order to solve the problem?

To fix this, we should pass the hidePickerModal as the value to the onBackdropPress prop. E.g

            <ValueSelectorModal
                isVisible={isPickerVisible}
                label={label}
                selectedItem={selectedItem}
                items={items}
                onClose={hidePickerModal}
                onItemSelected={updateInput}
                shouldShowTooltips={shouldShowTooltips}
                onBackdropPress={hidePickerModal}
            />

What alternative solutions did you explore? (Optional)

NA

[Pranav2612000]

@Krishna2323 This was added by you around 2 weeks back ( 904316b )
Was there a reason you used Navigation.dismissModal ?

[shahinyan11]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Room - Selection list RHP does not show sliding animation while being dismissed

What is the root cause of that problem?

The ValueSelectorModal opens separately in a RHP screen . And when we call Navigation.dismissModal in on backdrop press we the prev screen is closing with animation which will not affect to ValueSelectorModal

What changes do you think we should make in order to solve the problem?

when the user clicks on the background Call hidePickerModal too. To do that we can update this line like below

onBackdropPress={()=>{
    Navigation.dismissModal()
    hidePickerModal()
}}

What alternative solutions did you explore? (Optional)

Make the ValueSelectorModal separate screen in ModalStackNavigators and update onPress to navigate to this screen

[CortneyOfstad]

Pinged on the original PR here

[danieldoglas]

This should probably be fixed by the original PR creator, since it's in the regression period. @Krishna2323

[CortneyOfstad]

PR is still being reviewed 👍

[CortneyOfstad]

PR still in review 👍

[CortneyOfstad]

Bumped the PR as it looks like we're waiting on clarification on one comment

[rlinoz]

Reopening for proposals since we got to a dead end here.

[hoangzinh]

@rlinoz I think we should add back the "daily" label for this issue.

[CortneyOfstad]

Thanks @rlinoz!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[rlinoz]

waiting for proposals

[rlinoz]

waiting for proposals

[shahinyan11]

@rlinoz What about the suggestions above that you haven't reviewed yet?

[rlinoz]

@shahinyan11 which one specifically?

[shahinyan11]

@rlinoz this one

[rlinoz]

The proposal only works if we delay the dismiss: #38711 (comment)

And making it a separate screen was considered poor ROI before: #38711 (comment)

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[rlinoz]

I am inclined to close this for now, since we don't want to create the new screens yet.

[CortneyOfstad]

Sounds good to me @rlinoz! Closing!