You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Payment due][$1000] Room - Side panel does not close when clicking outside of panel #34394
Version Number: 1.4.24-4 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs:https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:
Action Performed:
Click on FAB > Start chat > room > workspace
Click outside of the side panel
Expected Result:
Side panel should close as it does when clicking on outside of the panel anywhere in the app
Actual Result:
Side panel stays open but navigates one step back
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
changed the title [-]Room - Side panel does not close when clicking outside of panel[/-][+][$500] Room - Side panel does not close when clicking outside of panel[/+]on Jan 11, 2024
What changes do you think we should make in order to solve the problem?
Option 1:
We need to modify ValueSelectorModal to accept a callback for onBackdropPress event also and then we will pass a function that calls Navigation.dismissModal() when onBackdropPress event is fired inside ValueSelectorModal. We also need to introduce onBackdropPress callback prop inside BaseModal.
Option 2:
Or we can use onBackButtonPress on HeaderWithBackButton to hide the valuePicker and use onClose to dismiss modal using Navigation.dismissModal() or Navigation.goBack(), inside ValueSelectorModal we can separate logic.
Option 3:
We will pass a callback to BaseModal directly from ValueSelectorModal that will run on backdropPress and will call Navigation.dismissModal() or Navigation.goBack(). Still we need to add a onBackdropPress prop inside BaseModal. We will be passing the callback to BaseModal as a default value. Like we did it here:
📣 @yuetong3yu! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:
Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
changed the title [-][$1000] Room - Side panel does not close when clicking outside of panel[/-][+][Payment due][$1000] Room - Side panel does not close when clicking outside of panel[/+]on May 15, 2024
@danieldoglas@kadiealexander@hoangzinh, IMO there should be no reduction in payment since the regression was minor and we ended up doing nothing there. I have put a lot of effort into this issue, first in this PR, then in the second PR, and then I have invested a lot of time into the PR to fix the regression, which had no straightforward solution because we didn't want to create separate screens. See #38665 (comment). Pls correct me if I'm wrong here 🙃
@danieldoglas I'll let you decide if the regression penalty should apply here, as I don't have the technical understanding to know if the regression was true.
Activity
[-]Room - Side panel does not close when clicking outside of panel[/-][+][$500] Room - Side panel does not close when clicking outside of panel[/+]melvin-bot commentedon Jan 11, 2024
Job added to Upwork: https://www.upwork.com/jobs/~011ab450f9ef4bcadd
melvin-bot commentedon Jan 11, 2024
Triggered auto assignment to @kadiealexander (
Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.melvin-bot commentedon Jan 11, 2024
Triggered auto assignment to Contributor-plus team member for initial proposal review - @hoangzinh (
External)Krishna2323 commentedon Jan 11, 2024
Proposal
Please re-state the problem that we are trying to solve in this issue.
Room - Side panel does not close when clicking outside of panel
What is the root cause of that problem?
We are only hiding picker modal when
onCloseis called insideValuePicker.App/src/components/ValuePicker/index.js
Lines 59 to 61 in 63ed47d
What changes do you think we should make in order to solve the problem?
Option 1:
We need to modify
ValueSelectorModalto accept a callback foronBackdropPressevent also and then we will pass a function that callsNavigation.dismissModal()whenonBackdropPressevent is fired insideValueSelectorModal. We also need to introduceonBackdropPresscallback prop insideBaseModal.Option 2:
Or we can use
onBackButtonPresson HeaderWithBackButton to hide thevaluePickerand useonCloseto dismiss modal usingNavigation.dismissModal()orNavigation.goBack(), insideValueSelectorModalwe can separate logic.Option 3:
We will pass a callback to
BaseModaldirectly fromValueSelectorModalthat will run onbackdropPressand will callNavigation.dismissModal()orNavigation.goBack(). Still we need to add aonBackdropPressprop insideBaseModal. We will be passing the callback toBaseModalas a default value. Like we did it here:App/src/libs/Navigation/AppNavigator/Navigators/RightModalNavigator.tsx
Lines 28 to 36 in b6a18b1
Result
workspace_modal_hide.mp4
Alternative:
We will make this a page on the RHP (Exactly like #26742). We will:
yuetong3yu commentedon Jan 11, 2024
Feel like it's expected instead a bug.
melvin-bot commentedon Jan 11, 2024
📣 @yuetong3yu! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:
Format:
yuetong3yu commentedon Jan 12, 2024
Contributor details
Your Expensify account email: yuetong3yu@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~019f5d35fda67374fb
melvin-bot commentedon Jan 12, 2024
✅ Contributor details stored successfully. Thank you for contributing to Expensify!
105 remaining items
kadiealexander commentedon May 15, 2024
Since there was a regression, reducing the payment by 50% as per the contributing guidelines.
Payouts due:
Upwork job is here.
[-][$1000] Room - Side panel does not close when clicking outside of panel[/-][+][Payment due][$1000] Room - Side panel does not close when clicking outside of panel[/+]Krishna2323 commentedon May 15, 2024
@danieldoglas @kadiealexander @hoangzinh, IMO there should be no reduction in payment since the regression was minor and we ended up doing nothing there. I have put a lot of effort into this issue, first in this PR, then in the second PR, and then I have invested a lot of time into the PR to fix the regression, which had no straightforward solution because we didn't want to create separate screens. See #38665 (comment). Pls correct me if I'm wrong here 🙃
kadiealexander commentedon May 15, 2024
@danieldoglas I'll let you decide if the regression penalty should apply here, as I don't have the technical understanding to know if the regression was true.
danieldoglas commentedon May 17, 2024
I agree we can't skip the reduction because of the effort applied.
kadiealexander commentedon May 20, 2024
Thanks Daniel!