text:
Reports - Chat filter is missing "In" field

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: 9.0.76-6
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both
If this was caught during regression testing, add the test name, ID and link from TestRail: Exp
Email or phone of affected tester (no customers): applausetester+kh04120001@applause.expensifail.com
Issue reported by: Applause Internal Team
Device used: Windows 11 / Chrome
App Component: Search
Action Performed:

Go to staging.new.expensify.com
Open any chat.
Click on the search icon on the top right.
Click "Search in" (make sure the search query is type:chat).
Type anything and hit Enter.
Click Filters.

Expected Result:
There should be "In" field because the search query is "in:x" (previously there was "In" row for chat filter, which can be seen in this issue https://github.com/Expensify/App/issues/52924).
Actual Result:
Chat filter is missing "In" field.
Workaround:
Unknown
Platforms:

[x] Android: Standalone
[x] Android: HybridApp
[x] Android: mWeb Chrome
[x] iOS: Standalone
[x] iOS: HybridApp
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos
https://github.com/user-attachments/assets/8f10af62-eea2-4eed-8da8-c0801672a83e
View all open jobs on GitHub

Proposal
Please re-state the problem that we are trying to solve in this issue.
Chat filter missing In filter field.
What is the root cause of that problem?
This happens after this PR. In that PR, a new filter "To" is added, but "In" filter is removed.
https://github.com/Expensify/App/blob/653af04f8bf51661eb18f1c48a56c1362e5c61b2/src/pages/Search/AdvancedSearchFilters.tsx#L208-L211
What changes do you think we should make in order to solve the problem?

If the removal of "In" filter isn't expected, then we can re-add it to the array.
https://github.com/Expensify/App/blob/653af04f8bf51661eb18f1c48a56c1362e5c61b2/src/pages/Search/AdvancedSearchFilters.tsx#L208-L211
Also, if the "To" filter isn't expected for Chats, then we can remove it. Because currently, "To" filter only shows results if we select ourself. Looks like "To" filters any chat that is sent to the user selected on the filter, and we can only do it for ourself.
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

The filter key is a CONST object, so not sure if testing the content of a const is necessary, but we can definitely add it if needed..

authors: IuliiaHerets bernhardoj


text:
[HOLD for payment 2022-01-27] Chat moves up a bit when new message is received - Reported by @K4tsuki

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:

Open a room and scroll to earlier message (fastly)
Stay on specific message (to read it  or something)
Send new message to that room
Notice chat view is moving around when new message is received

Expected Result:
User should stay at the same position
Actual Result:
User moves up by a size of the new message, when it is received
Not repro on Web/Desktop
Workaround:
Unknown
Platform:

Where is this issue occurring?

iOS
Android

Version Number: 1.1.27-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/43995119/149064955-22fdc8a9-f8bc-4029-a558-6290a5243fac.MP4
Expensify/Expensify Issue URL:
Issue reported by: @K4tsuki
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1641188993370400
View all open jobs on GitHub

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

I am fine if this issue is duplicate of https://github.com/Expensify/App/issues/6491, because fixing that issue might be fixing this issue too.

I think this should weekly because it's not too critical. Here is another video showing the bug on IOS.
https://user-images.githubusercontent.com/26260477/149182902-b856c778-ea7f-4c9b-865d-15c275cf1889.mp4

On it!

Posted!
Internal: https://www.upwork.com/ab/applicants/1482023251637559296/job-details
External: https://www.upwork.com/jobs/~0195002504c8fcf973

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

Triggered auto assignment to @mountiny (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

Proposal for IOS only
Adding maintainVisibleContentPosition prop to FlatLIst solve this issue or IOS.
Code change
src/components/InvertedFlatList/BaseInvertedFlatList.js
  <FlatList
                // eslint-disable-next-line react/jsx-props-no-spreading
                {...this.props}
                ref={this.props.innerRef}
                inverted
                renderItem={this.renderItem}

                // Native platforms do not need to measure items and work fine without this.
                // Web requires that items be measured or else crazy things happen when scrolling.
                getItemLayout={this.props.shouldMeasureItems ? this.getItemLayout : undefined}

                // We keep this property very low so that chat switching remains fast
                maxToRenderPerBatch={1}
                windowSize={15}
                removeClippedSubviews={this.props.shouldRemoveClippedSubviews}
+                maintainVisibleContentPosition={{minIndexForVisible: 0}}
            />

https://user-images.githubusercontent.com/21364901/149600512-999970a6-0d9c-493d-b54c-24c92eabae2d.mov

for android, there is pull request on react native still not merged yet
so the previous solution will work also on android when it gets merged in the next versions.
https://github.com/facebook/react-native/pull/29466

Nice find! It might take months for RN to merge that PR (29466). And then we'll have to upgrade RN, or merge the changes into a fork. Until then we can keep this issue on hold.
Alternative proposals which also fix the root cause (ie not workarounds) are welcome!
cc: @mountiny what do you think?

Thank you for the review @rushatgabhane. I think it is better to fix this for iOS when we have a simple tool for it and then just keep an eye on the RN PR so we can update whenever that goes out for Android too. Do you agree or are there any reasons why it might be better to hold for the feature to be finished in Android too?

@mountiny my bad, this proposal needs work. It's causing this janky animation when sending a message.

I think it is better to fix this for iOS when we have a simple tool for it and then just keep an eye on the RN PR so we can update whenever that goes out for Android too

Yep, I agree. If we go this route, no need to hold.
cc: @ahmdshrif
https://user-images.githubusercontent.com/29673073/149643066-a220fb27-4cf0-4e4d-a06b-d6f0cdb43c1f.mov

@rushatgabhane  just adding  autoscrollToTopThreshold: 0  will solve this issue
maintainVisibleContentPosition={{minIndexForVisible: 0, autoscrollToTopThreshold: 0}}

https://user-images.githubusercontent.com/21364901/149667812-8a6e9aad-22b9-42c2-b836-04b1c219a77f.mov

🎀👀🎀 C+ Reviewed
@ahmdshrif's proposal looks good! We'll fix it for Android by upgrading RN in future. I'll keep an eye on it.
cc: @mountiny

📣 @ahmdshrif You have been assigned to this job by @mountiny!
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 📖

Great! Thank you for the review Rushat!
@ahmdshrif Can you please apply for the job on Upwork and create the PR? Thank you very much!

yes already applied
my account https://www.upwork.com/freelancers/~0143a705fba636cfd8

can you please hire me @dylanexpensify?

@ahmdshrif offer sent! 😄

We have discovered the solution has an edge where it still fails, since the issue description originally has not stated mobile Web as a platform where the issue persists, we should not hold that as a regression. I have created a follow-up issue to look into this problem which occurs only on safari mWeb iOS for some reason. @ahmdshrif has looked into it already in the Pr and could not find a straightforward solution.

thanks, @mountiny, I am happy to do my first merged Pr here.
do I need to wait for more steps or I can submit work now?

do I need to wait for more steps or I can submit work now?

If there isn't any regression in staging and production, your contract will be paid out. There's more info on it in contribution.md
I hope this answers your question!

ok, thanks @rushatgabhane

@ahmdshrif Likewise, it was very lovely to work with you and I hope this was not our last mutual PR 😊
I would recommend you to look around the Help Wanted labelled issues and see if there is anything you might be interested in. However, you should wait til your PR is deployed to production before writing proposals. I think that should not take long given it is in staging and the QA has passed (except the nasty edge case).

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

https://github.com/Expensify/App/pull/7271

If no regressions arise, payment will be issued on 2022-01-27. :confetti_ball:

@dylanexpensify Should I apply to upwork job too for reporting this issue?

Sending payment now, @K4tsuki I'll create a job for you to apply to for the reporting!!

@ahmdshrif payment sent and contract ended!
@K4tsuki please apply here for reporting issue! https://www.upwork.com/jobs/~01a90138a351b9386f

Hi
The solution for this issue has been deployed to production  in version 1.1.31-1 and is now subject to a 7-day regression period

@dylanexpensify I have applied to the upwork job.

Sent the offer @K4tsuki !

@dylanexpensify I have accepted the offer. Thank you.

Payment sent, contract ended, and job closed!

source: gharchive/issue

authors: K4tsuki MelvinBot ahmdshrif botify dylanexpensify mountiny mvtglobally neil-marcellini promarat rushatgabhane

repo: Expensify/App

license: MIT

license_type: permissive

license_source: github-api


text:
[$250] Settings - Payment options are overlapping with Add Payment Method under Settings

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:

Log in to NewDot with any account
Go Setting - Payments
Tap on the middle "Add Payment Method"

Expected Result:
Payment options are not overlapping with Add Payment Method under Settings
Actual Result:
Payment options are overlapping with Add Payment Method under Settings
Workaround:
Visual
Platform:

Where is this issue occurring?

Web
Desktop App

Version Number: 1.1.70.2
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:

https://user-images.githubusercontent.com/43996225/171507737-2ce1c6f5-63dc-496b-86a9-c8543dca8528.mp4
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:
View all open jobs on GitHub

Job post: https://www.upwork.com/jobs/~014470d4b94fc99a28

Proposal
Problem
We're using event.target.getBoundingClientRect to get the placement position of the dropdown. The target can be any child of the element to which the listener is attached. As such, we're getting incorrect positions.
Solution
We should use event.currentTarget.getBoundingClientRect instead to get the correct element position.
The following changes:
https://github.com/allroundexperts/Expensify/blob/1b74843140f609fa8900d754148fa9e9e7009176/src/libs/getClickedElementLocation/index.js#L8
To
    return nativeEvent.currentTarget.getBoundingClientRect();

Result
https://user-images.githubusercontent.com/30054992/171672913-23d7aa42-3b2d-4cb8-90a2-f5e9fbdb7392.mov

Proposal
To keep the position of the menu when resizing the window.

Add styles.pointerEventsNone to container of childrens inside the . Line 198
https://github.com/Expensify/App/blob/d2f9d6124a50b8420ce1378a2a3c9422df691c18/src/components/Button.js#L198

<View style={[styles.justifyContentBetween, styles.flexRow, styles.pointerEventsNone]}>
....
</View>

Preview

Without window resize & with window resize

https://user-images.githubusercontent.com/5449119/171684157-3dd3ec74-2775-4913-8765-bdad9051a1b8.mp4

Hey @mananjadhav , maybe you are wrong, by adding styles.pointerEventsNone in the renderContent which renders inside the Button it won't affect any cursor options for the Button itself.
https://github.com/Expensify/App/blob/72aac1c027fa8ff20ba2aa966680a5afda4dab2c/src/components/Button.js#L272
However, the Button is wrapped by Pressable and OpacityView, see:
https://github.com/Expensify/App/blob/72aac1c027fa8ff20ba2aa966680a5afda4dab2c/src/components/Button.js#L230
https://github.com/Expensify/App/blob/72aac1c027fa8ff20ba2aa966680a5afda4dab2c/src/components/Button.js#L252
Sorry but the proposal of @allroundexperts only gets the position of Button once. It will cause an issue that I fixed before when the user resizes the window width of the browser, see:
Issue: https://github.com/Expensify/App/issues/7385
Fixed by: https://github.com/Expensify/App/pull/7847 & https://github.com/Expensify/App/pull/7566

@phivh This isn't the root of the problem. The root is a wrong usage of target. Also, I'm not sure how this will effect the fix you made.

Proposal
Problem
As shared by @allroundexperts the root cause of this is the use of target instead of currentTarget in the function getClickedElementLocation method call.
event.currentTarget tells us on which element the event was attached or the element whose eventListener triggered the event whereas event.target tells where the event started.
However, there are two problems with @allroundexperts' solution:

setMenuPosition registered at Dimensions.addEventListener('change', this.setMenuPosition) throws an error when the browser is resized:

It doesn't handle the resize event of the browser - meaning, if the payment method pop-up is visible and the browser is resized, the pop-up stays in the same place.

Solution
The fix for this issue is a two part solution:

To store the current target from the native event inside the state for later use
A new folder for getClickedTargetLocation which takes care of the web and native platforms and returns the getBoundingClientRect from the passed target stored in step 1.

paymentMethodPressed(nativeEvent, accountType, account, isDefault) {
   // ++ Changes starts
    const position = getClickedTargetLocation(nativeEvent.currentTarget);

    this.setState({
        addPaymentMethodNativeEventCurrentTarget: nativeEvent.currentTarget,
        addPaymentMethodButton: nativeEvent,
    });
   // ++ Changes Ends

    if (accountType) {
        let formattedSelectedPaymentMethod;
        if (accountType === CONST.PAYMENT_METHODS.PAYPAL) {
            formattedSelectedPaymentMethod = {
                title: 'PayPal.me',
                icon: account.icon,
                description: account.username,
                type: CONST.PAYMENT_METHODS.PAYPAL,
            };
        } else if (accountType === CONST.PAYMENT_METHODS.BANK_ACCOUNT) {
            formattedSelectedPaymentMethod = {
                title: account.addressName,
                icon: account.icon,
                description: `${this.props.translate('paymentMethodList.accountLastFour')} ${account.accountNumber.slice(-4)}`,
                type: CONST.PAYMENT_METHODS.BANK_ACCOUNT,
            };
        } else if (accountType === CONST.PAYMENT_METHODS.DEBIT_CARD) {
            formattedSelectedPaymentMethod = {
                title: account.addressName,
                icon: account.icon,
                description: `${this.props.translate('paymentMethodList.cardLastFour')} ${account.cardNumber.slice(-4)}`,
                type: CONST.PAYMENT_METHODS.DEBIT_CARD,
            };
        }
        this.setState({
            isSelectedPaymentMethodDefault: isDefault,
            shouldShowDefaultDeleteMenu: true,
            selectedPaymentMethod: account,
            selectedPaymentMethodType: accountType,
            formattedSelectedPaymentMethod,
        });
        this.setPositionAddPaymentMenu(position);
        return;
    }
    this.setState({
        shouldShowAddPaymentMenu: true,
    });

    this.setPositionAddPaymentMenu(position);
}

And change the setMenuPosition function as below:
setMenuPosition() {
        if (!this.state.addPaymentMethodNativeEventCurrentTarget) {
            return;
        }
        const buttonPosition = getClickedTargetLocation(this.state.addPaymentMethodNativeEventCurrentTarget);
        this.setPositionAddPaymentMenu(buttonPosition);
    }

I also set the addPaymentMethodNativeEventCurrentTarget to null when the pop-up is dismissed:
hideAddPaymentMenu() {
        this.setState({
            addPaymentMethodNativeEventCurrentTarget: null,
            shouldShowAddPaymentMenu: false
        });
    }

Attached is the screencast (with voice) showing the issue with browser resize and the applied fix:
https://user-images.githubusercontent.com/911879/172020231-a1fb0629-6387-4752-b23c-8b6a2c50aa5e.mp4
@mananjadhav kindly review my proposal.
Thank you!

Apparently the screencast uploaded in the previous message doesn't seem to play. You may view it here: https://www.veed.io/view/3dda4edd-5079-4314-8683-1386e373ac05

The browser resize issue exists with the original implementation as well. No where in the ticket, was it mentioned that responsiveness needs to be fixed.The ticket and the video posted, mention the issue which was fixed in my PR.

Sure, no offence please @allroundexperts.
Your did still introduce the javascript error as shared in my proposal and so I opted to propose a new solution. Cheers :)

@liyamahendra Thanks for pointing out the error. We can easily fix it by changing the getClickedElementLocation to:
function getClickedElementLocation(nativeEvent) {
    return (nativeEvent.currentTarget || nativeEvent.target).getBoundingClientRect();
}
```. I've updated my proposal to handle that as well.

The browser resize issue exists with the original implementation as well. No where in the ticket, was it mentioned that responsiveness needs to be fixed. The ticket and the video posted, mention the issue which was fixed in my PR.

I was wrong here. The original version WAS responsive. It was my change which introduced the bug that @liyamahendra pointed out. I've updated my proposal to handle that bug. I still feel like my solution is more simpler and to the point.

Just catching up as there is a lot of back and forth and I'm not sure if I understand the root cause.

@allroundexperts's proposal looks good to me. I can see this isn't used at other places except for KYCWall within the Payments Screen. @marcaaron What are your thoughts?

One question I have about this solution is that we are not being very specific about the html element we are targeting and why we are targeting it. Why would we use target sometimes and currentTarget other times and not one or the other? More explanation is needed.
@phivh I'm also not getting where we are disabling pointer events and why it resolves the issue. Can you give some explanation?
@mananjadhav Please ask more clarifying questions before accepting a proposal.

@marcaaron did you have a chance to review my proposal? Would love to hear your comments please!

Just catching up as there is a lot of back and forth and I'm not sure if I understand the root cause.

@allroundexperts's proposal looks good to me. I can see this isn't used at other places except for KYCWall within the Payments Screen. @marcaaron What are your thoughts?

One question I have about this solution is that we are not being very specific about the html element we are targeting and why we are targeting it. Why would we use target sometimes and currentTarget other times and not one or the other? More explanation is needed.
@phivh I'm also not getting where we are disabling pointer events and why it resolves the issue. Can you give some explanation?
@mananjadhav Please ask more clarifying questions before accepting a proposal.

@marcaaron We're using target only when currentTarget is undefined. This is to handle the case that @liyamahendra pointed out.

@marcaaron I'll explain why we need to use currentTarget and not target to address the root cause of this issue.
The "Add payment method" is a composite control (if I may say so) consisting of the text label wrapped inside a Button. This issue specifically occurs when the click is on the text of the button. If the click is anywhere in the green background, the issue doesn't occur. This can be seen from the below screencast:
https://user-images.githubusercontent.com/911879/172217470-6356d94d-a499-43a7-8e59-14de0dfaaef6.mp4
Now, the click event handler is attached to the button and not to the text.  event.currentTarget tells us on which element the event was attached or the element whose eventListener triggered the event whereas event.target tells where the event started.
The code was earlier using nativeEvent.target which makes it clear that if you click on the text of the button, the event started from the text and so the pop-up would be positioned below the text label, which is what is happening. Since we need to show the pop-up below the button, we need to use the currentTarget which is where the event is attached.
BTW, my solution only uses currrentTarget.

I see that the screencast showing the issue and my fix which I uploaded on a 3rd party service is no longer available today. Uploading the screencast on YouTube. Here is the link to it: https://www.youtube.com/watch?v=PmpMYSfhKuQ
@marcaaron @mananjadhav please share your comments and review my proposal! Thank you!

Will review the new comments/questions and update them here.

waiting for @mananjadhav

@mananjadhav can you help us move this along? Thanks.

@liyamahendra I understand that we might have to use currentTarget. Overall your proposal looks good but I have a few questions on your proposal before we finalize.

What does the body of getClickedTargetLocation look like for web and native. Can you share that please?
Are you suggesting we discard getClickedElementLocation? and use only getClickedTargetLocation?
getClickedElementLocation is also used in BaseKYCWall, so I am assuming we will also have to update that component as it might have the same problem?
I can see addPaymentMethodButton in the state is used for the purpose of finding the position. Do we still need it after your proposed change of adding  addPaymentMethodNativeEventCurrentTarget. Can we just retain addPaymentMethodButton state with your new variable's value?

Thank you for your time and sorry for the delay.

Thank you @mananjadhav for getting back on this!

New folder src/libs/getClickedTargetLocation with below two files (one for web & another one for native):

index.js
/**
 * Returns the Bounding Rectangle for the passed native event's target.
 *
 * @param {Object} target
 * @returns {Object}
 */
function getClickedTargetLocation(target) {
    return target.getBoundingClientRect();
}

export default getClickedTargetLocation;

index.native.js
/**
 * We don't need to get the position of the element on native platforms because the popover will be bottom mounted
 *
 * @returns {Object}
 */
function getClickedTargetLocation() {
    return {
        bottom: 0,
        left: 0,
    };
}

export default getClickedTargetLocation;

Yes because getClickedElementLocation ins't used inside BasePaymentsPage screen anywhere else.
Would you please share insights into how can I test BaseKYCWall. Then, I'll make sure I update getClickedElementLocation as required.
Ideally, if we go ahead with my changes, addPaymentMethodButton won't be required. Can be safely removed from state.

Please let me know your comments!

@mananjadhav Whats wrong with my proposal?

Thanks for the clarifying comments @liyamahendra. I am fine with your proposal but I feel we're making too many unwanted changes.  We can retain getClickedElementLocation and change the parameter to target as your function describes. Secondly, we simplify the BasePaymentsPage with just these changes as discussed in the above comments.
       const position = getClickedElementLocation(nativeEvent.currentTarget);
        this.setState({
            addPaymentMethodButton: nativeEvent.currentTarget,
        });

What I am recommending is, instead of removing all current variables and adding functions, lets reuse the current ones and update the body/value wherever required.
@marcaaron I feel @liyamahendra's proposal is good to go. Let me know your thoughts.

Whats wrong with my proposal?

@allroundexperts Having currentTarget || target condition will lead to unexpected behaviour. Additionally, if you try the solution you'll see it breaks when resizing. Because we don't have the reference to currentTarget all the time, it picks target and ends up repositioning the menu. Attached is the video applied with your fix.
https://user-images.githubusercontent.com/3069065/173933364-300ce7b6-9ca2-4ac8-80f9-ba4c1c93258f.mov

@marcaaron I am reviewing this proposal for the issue  where we're trying to get the "Add Payment Method" outside the flat list. If we were to do that, we might not need this issue to be fixed. The button will be docked at the button and the Popovermenu will always open above like the "IOU Payment" button.

Good call @mananjadhav. Let's move the discussion to one issue. There is also this https://github.com/Expensify/App/issues/7375 issue where we are trying to fix the popover for Native devices, especially for Tablets.
But I think https://github.com/Expensify/App/issues/7375 is good as a separate issue it needs a few unrelated changes.

@parasharrajat @mananjadhav There is another issue which effecting the Add Payment Method and Delete button. Do you want us to create separate issue ?
https://user-images.githubusercontent.com/93399543/174348206-c9360efe-fdc7-46ae-a912-163d9b726b42.mp4

@mananjadhav and @parasharrajat I see https://github.com/Expensify/App/issues/7375 issue mentions about https://github.com/Expensify/App/issues/9283 and vice versa.

@marcaaron I feel @liyamahendra's proposal is good to go. Let me know your thoughts.

Since @mananjadhav reviewed my proposal to this issue, just wanted to check the status here. @marcaaron Do we plan to fix this or just close it?

@liyamahendra Lets wait, I've been testing the other proposal and it seems the issue of the overlap still exists even if we push the button on the bottom. So we might be able to solve them separately.

@marcaaron Can you help us with the way forward?

@parasharrajat @mananjadhav There is another issue which effecting the Add Payment Method and Delete button. Do you want us to create separate issue ?

@kbecciv I am guessing the issue is that when the "Add Payments" popup is open, it doesn't show the "Delete Payment" on the first click. If thats the issue then yeah it would be separate issue.

@mananjadhav "Delete payment" does show on the first click. Its about the overlapping of "Delete"

https://user-images.githubusercontent.com/43996225/174644319-0d84e81f-372c-4c7b-9832-58915a26f153.mp4
button on "add payment" button. Attaching video and screenshot.

Yeah this won't be a problem if the "Add Payment" button is moved to the bottom in #8929

@marcaaron The issue still exists even if we move the button to the bottom. @liyamahendra's proposal is good to go with the feedback https://github.com/Expensify/App/issues/9283#issuecomment-1156966917 here.

@marcaaron @mananjadhav Please let me know if you guys would like me to raise a PR.

@liyamahendra Please wait for the proposal to be accepted and to be hired (atleast here on GH). You can then start with the PR. Thank you for your patience.

@liyamahendra Can you please summarize in a new comment the complete proposal?

Perfect, thanks!

@marcaaron I have applied for the job on Upwork. Have created the PR here

Gladly! :) Adding the reviewing label as there's a PR up for this.

👋 @liyamahendra can we get a status update on the linked PR please? What's left to do and when do you expect to be able to get it over the line? Thanks!

I think @mananjadhav gave his clearance here. There were a couple of comments after that from @marcaaron which were resolved as well.
Maybe @mananjadhav or @marcaaron can provide a status update if there is anything pending.

Bump @mananjadhav & @marcaaron shouldn't this PR be merged by now? 🤔

@marcaaron Wanted to check something before we merge this. If there's any changes based on his feedback, @liyamahendra and I would take this up.

Sorry, I'm not sure what is needed from me. What is it exactly that you wanted to check?

Seems like we are waiting for @liyamahendra to request another review.

I think we probably must consider this abandoned? It's been a month without an update.

Seems like we are waiting for @liyamahendra to request another review.

@marcaaron sorry, I'm not aware what are we waiting for here from me. Happy to do the needful if you please let me know. Thanks

@marcaaron I had reviewed the PR and approved. Waiting for your final approval.

@mananjadhav will you review the changes again?

My expectations here are that @liyamahendra will let us know when the PR is ready for re-review and that @mananjadhav responds to the review request. So, that's what I've been waiting for :)

@marcaaron the PR is ready for review. @mananjadhav please do the needful!

I'll do that today.

Issue not reproducible during KI retests. (First week)

@trjExpensify Quick bump on Upwork payment.

Yep, I've been OOO. Sorry! So looking at the Upwork job, we've already settled up with @liyamahendra a while back. @NicMendonca might have some insight into that, but seems like we're settled:

As for C+ payment @mananjadhav, I've settled up with you.
Going to close this out, but @liyamahendra of course reopen if I didn't get something right here.

authors: NicMendonca allroundexperts kavimuru kbecciv liyamahendra mananjadhav marcaaron mvtglobally parasharrajat phivh trjExpensify


text:
Inbox -Using "Enter" Key in request Call modal causing modal reopening every time, blocking main page

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

Coming from this /Expensify repo issue cc @kavimuru
Action Performed:

Go to staging.expensify.com
Log in with any  account
Go to Accounts - Accounts details
On Secondary Logins page enter the phone number +17076414722 and saved it
Go back to Inbox
Use Enter Key to call Concierge

Expected Result:
Able to Request a Call by using Enter Key
Actual Result:
Using Enter Key in request Call modal causing page opens every time, blocking main page
Workaround:
Unknown
Notes/Photos/Videos:
https://user-images.githubusercontent.com/43996225/176504177-408ef894-a2d3-4d6a-9298-9bc5e0382a77.mp4
Tester login credentials - Email/Phone: applausetester+062922abb@applause.expensifail.com
Tester login credentials - Password: Feya86Katya
View all open jobs on GitHub

How is this affecting NewDot?

Shoot, you are right @parasharrajat .. I got confused by the external label my mistake

authors: MitchExpensify parasharrajat


text:
[CP Staging] Fix Success / Danger Button Text Color, Fix Growl Colors

Details

Fixed Issues

$ https://github.com/Expensify/App/issues/12822
Tests

[x] Verify that no errors appear in the JS console
[x] Verify all buttons are the correct style

QA Steps

[ ] Verify that no errors appear in the JS console
[ ] Verify all buttons have the correct text color

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above

[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms

[x] I ran the tests on all platforms & verified they passed on:

[x] iOS / native
[x] Android / native
[x] iOS / Safari
[x] Android / Chrome
[x] MacOS / Chrome
[x] MacOS / Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)

[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product was added in all src/languages/* files
[x] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers

[x] I followed the guidelines as stated in the Review Guidelines

[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)

[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)

[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such

[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)

[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.

[x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

PR Reviewer Checklist
The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed

[ ] I have verified the author checklist is complete (all boxes are checked off).
[ ] I verified the correct issue is linked in the ### Fixed Issues section above
[ ] I verified testing steps are clear and they cover the changes made in this PR

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for expected offline behavior are in the Offline steps section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[ ] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[ ] I checked that screenshots or videos are included for tests on all platforms
[ ] I included screenshots or videos for tests on all platforms
[ ] I verified tests pass on all platforms & I tested again on:

[ ] iOS / native
[ ] Android / native
[ ] iOS / Safari
[ ] Android / Chrome
[ ] MacOS / Chrome
[ ] MacOS / Desktop

[ ] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[ ] I verified that comments were added to code that is not self explanatory
[ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[ ] I verified any copy / text shown in the product was added in all src/languages/* files
[ ] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[ ] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[ ] All JSX used for rendering exists in the render method
[ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[ ] If a new CSS style is added I verified that:

[ ] A similar style doesn't already exist
[ ] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots

Web

still works for dark switch

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

Whoops idk what just happened with puller bear, I didn't mean to remove

Web - all the buttons seems to look great! Great quick fix @grgia 🎉

oh wait in the drop-down button it's black, is that expected?

iOS also great!

Reviewer Checklist

[ ] I have verified the author checklist is complete (all boxes are checked off).
[ ] I verified the correct issue is linked in the ### Fixed Issues section above
[ ] I verified testing steps are clear and they cover the changes made in this PR

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[ ] I checked that screenshots or videos are included for tests on all platforms
[ ] I included screenshots or videos for tests on all platforms
[ ] I verified tests pass on all platforms & I tested again on:

[ ] iOS / native
[ ] Android / native
[ ] iOS / Safari
[ ] Android / Chrome
[ ] MacOS / Chrome
[ ] MacOS / Desktop

[ ] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[ ] I verified that comments were added to code that is not self explanatory
[ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[ ] I verified any copy / text shown in the product was added in all src/languages/* files
[ ] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[ ] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[ ] All JSX used for rendering exists in the render method
[ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[ ] If a new CSS style is added I verified that:

[ ] A similar style doesn't already exist
[ ] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

@mountiny I'm gonna look into that dropdown button really quick

@grgia I agree, noticed there are some black lines, lets fixed that first

@mountiny Should be Fixed!

Merged this so the actions should pass now https://github.com/Expensify/App/pull/12831

@grgia Can you merge in main to get the new test fix?

👋 the buttons are looking good for me again, but I noticed the growls are white (background and text).
https://user-images.githubusercontent.com/87341702/202559495-4f312122-c752-434c-93c1-ee14ec8d6901.mov
I'm not sure if this is related, but you may know better than me.

Will merge main, and look at growls now!

Oh yeah that looks realted. I will have to head out soon so @aldo-expensify feel free to take over with merging :)

Oh yeah that looks realted. I will have to head out soon so @aldo-expensify feel free to take over with merging :)

Sure, no problem!

I have been testing random things, including offline behaviour and errors and I haven't spotted anything else beside the growl.

@aldo-expensify could you send a picture of the correct growl colors?

@aldo-expensify could you send a picture of the correct growl colors?

looking for it!

Does this help?

or do you need like the hex codes?

that's perfect! Latest commits should fix

Reviewer Checklist

[ ] I have verified the author checklist is complete (all boxes are checked off).
[ ] I verified the correct issue is linked in the ### Fixed Issues section above
[ ] I verified testing steps are clear and they cover the changes made in this PR

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[ ] I checked that screenshots or videos are included for tests on all platforms
[ ] I included screenshots or videos for tests on all platforms
[ ] I verified tests pass on all platforms & I tested again on:

[ ] iOS / native
[ ] Android / native
[ ] iOS / Safari
[ ] Android / Chrome
[ ] MacOS / Chrome
[ ] MacOS / Desktop

[ ] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[ ] I verified that comments were added to code that is not self explanatory
[ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[ ] I verified any copy / text shown in the product was added in all src/languages/* files
[ ] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[ ] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[ ] All JSX used for rendering exists in the render method
[ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[ ] If a new CSS style is added I verified that:

[ ] A similar style doesn't already exist
[ ] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

I think we can go ahead then! Thanks for fixing and reviewing this 🙌

🚀 Cherry-picked to staging by @mountiny in version: 1.2.29-4 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 1.3.28-2 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Deployed to production by https://github.com/AndrewGable in version: 1.3.28-5 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

source: gharchive/pull-request

authors: OSBotify aldo-expensify grgia mountiny


text:
🍒 Cherry pick PR #19294 to staging 🍒

🍒 Cherry pick https://github.com/Expensify/App/pull/19294 to staging 🍒

This pull request has merge conflicts and can not be automatically merged. :disappointed:
Please manually resolve the conflicts, push your changes, and then request another reviewer to review and merge.
Important: There may be conflicts that GitHub is not able to detect, so please carefully review this pull request before approving.

🚀 Cherry-picked to staging by https://github.com/madmax330 in version: 1.3.17-2 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Deployed to production by https://github.com/chiragsalian in version: 1.3.17-5 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 1.3.28-2 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Deployed to production by https://github.com/AndrewGable in version: 1.3.28-5 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅


text:
🍒 Cherry pick PR #21029 to staging 🍒

🍒 Cherry pick https://github.com/Expensify/App/pull/21029 to staging 🍒

This pull request has merge conflicts and can not be automatically merged. :disappointed:
Please manually resolve the conflicts, push your changes, and then request another reviewer to review and merge.
Important: There may be conflicts that GitHub is not able to detect, so please carefully review this pull request before approving.

Reviewer Checklist

[x] I have verified the author checklist is complete (all boxes are checked off).
[x] I verified the correct issue is linked in the ### Fixed Issues section above
[x] I verified testing steps are clear and they cover the changes made in this PR

[x] I verified the steps for local testing are in the Tests section
[x] I verified the steps for Staging and/or Production testing are in the QA steps section
[x] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[x] I checked that screenshots or videos are included for tests on all platforms
[x] I included screenshots or videos for tests on all platforms
[x] I verified tests pass on all platforms & I tested again on:

[x] Android / native
[x] Android / Chrome
[x] iOS / native
[x] iOS / Safari
[x] MacOS / Chrome / Safari
[x] MacOS / Desktop

[x] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[x] I verified proper code patterns were followed (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I verified that this PR follows the guidelines as stated in the Review Guidelines
[x] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

🚀 Cherry-picked to staging by https://github.com/chiragsalian in version: 1.3.29-4 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Deployed to production by https://github.com/luacmartins in version: 1.3.29-11 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

authors: OSBotify chiragsalian


text:
fix: replace new report with preexisting

Details

This PR handles the case where the client attempts to create a new DM/group-DM with users they already have a DM/group-DM with. This can happen if:

You attempt to DM someone's secondary login when you already have a DM with their primary login
You attempt to DM an email address of someone you don't "know" who you've DM'd before using their accountID

Fixed Issues

$ https://github.com/Expensify/App/issues/15511
Tests

Create an account (userA) that has both a primary and secondary login
From accountB send a message to userA using userA's primary login
From accountB create a new chat with userA using their secondary login
Once the API responds (may take a moment), you should be re-routed to the existing chat with userA as long as you are still on the optimistically created chat with userA. Otherwise, you should see the optimistically created chat disappear from the LHN

[ ] Verify that no errors appear in the JS console

Offline tests

This cannot be done offline since it requires an API response. However if you create the report while offline, once you come back online, step 4 from above should still happen.
QA Steps

Create an account (userA) that has both a primary and secondary login
From accountB send a message to userA using userA's primary login
From accountB create a new chat with userA using their secondary login
Once the API responds (may take a moment), you should be re-routed to the existing chat with userA as long as you are still on the optimistically created chat with userA. Otherwise, you should see the optimistically created chat disappear from the LHN

[x] Verify that no errors appear in the JS console

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] Android / native
[x] Android / Chrome
[x] iOS / native
[x] iOS / Safari
[x] MacOS / Chrome / Safari
[x] MacOS / Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome
https://github.com/Expensify/App/assets/23215729/f55d392e-f799-4831-bf68-ec542ba0e73f

Mobile Web - Safari
https://github.com/Expensify/App/assets/23215729/5b3247a5-02ac-4773-b5ab-b19769a15605

Desktop
https://github.com/Expensify/App/assets/23215729/639c1394-82a9-495e-8a42-e2fdad29b6b4

iOS
https://github.com/Expensify/App/assets/23215729/71e59965-6d7f-411d-aede-74a525cb3259

Android
https://github.com/Expensify/App/assets/23215729/888b4dff-050c-4a96-aa3c-bc0b475fae1e

Thanks for this @pasyukevich. Can you help me understand conceptually the difference between Navigation.TYPE and Navigation.ACTION_TYPE and what the difference between UP and FORCED_UP is?

Also, looks like your commits need to be signed.

Sure,
TYPE - navigation types for linkTo function
ACTION_TYPE - navigation types from the react-navigation
'UP' is already used over the app and make replace action
'FORCE_UP' is new one to add additional case for replace action
Currently,  it is not possible to run 'UP' in some cases, cause the previous statement is  if (action.payload.name === NAVIGATORS.CENTRAL_PANE_NAVIGATOR && getTopmostReportId(root.getState()) !== getTopmostReportId(state)))
To handle this without the problems with the current logic - 'FORCE_UP' was added

Got it, thanks for the explanation.
@mananjadhav can you give it the first review?
Also @pasyukevich looks like you have a conflict.

Conflict resolved

Thanks, I am going to be reviewing this today.

@pasyukevich I had done a partial review, earlier and I see we're force pushing the changes everytime. Any specific reason we're force pushing the changes?

Checked the code, and I also tested this on the Web works fine. Finishing other platforms now.

Reviewer Checklist

[x] I have verified the author checklist is complete (all boxes are checked off).
[x] I verified the correct issue is linked in the ### Fixed Issues section above
[x] I verified testing steps are clear and they cover the changes made in this PR

[x] I verified the steps for local testing are in the Tests section
[x] I verified the steps for Staging and/or Production testing are in the QA steps section
[x] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[x] I checked that screenshots or videos are included for tests on all platforms
[x] I included screenshots or videos for tests on all platforms
[x] I verified tests pass on all platforms & I tested again on:

[ ] Android / native
[x] Android / Chrome
[x] iOS / native
[x] iOS / Safari
[x] MacOS / Chrome / Safari
[x] MacOS / Desktop

[x] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[x] I verified proper code patterns were followed (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I verified that this PR follows the guidelines as stated in the Review Guidelines
[x] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
https://github.com/Expensify/App/assets/3069065/9eac7817-4275-441a-b871-d3d754a29724

Mobile Web - Chrome
https://github.com/Expensify/App/assets/3069065/5414a623-1f3a-4107-98ba-8b56b36d461e

Mobile Web - Safari
https://github.com/Expensify/App/assets/3069065/d7cc3f7a-b743-46f7-a3bb-753a72743bd7

Desktop
https://github.com/Expensify/App/assets/3069065/808b4c2a-625d-4a4a-8826-774289701a9b

iOS
https://github.com/Expensify/App/assets/3069065/a6f754d0-7ee5-4249-ba86-0295a86f1423

Android

My android build is still running. Will post the video as soon as it is done.
But quick question @pasyukevich  @puneetlath, the flow works good overall but do you feel the transition is a bit janky? There is no feedback to the user who's being redirected to another page. I am not sure how relevant is it for the end users?

I agree it's a bit abrupt, but I think that's ok for now. We can always provide more visual feedback if we think it's needed in the future.

@mananjadhav There wasn't any specific reason, just to resolve conflict without additional commits
Regarding transition, I also agree that it can be improved in future

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

🚀 Deployed to staging by https://github.com/puneetlath in version: 1.3.55-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to staging by https://github.com/puneetlath in version: 1.3.56-0 🚀

platform
result

🤖 android 🤖
failure ❌

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/roryabraham in version: 1.3.56-24 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅

authors: OSBotify mananjadhav pasyukevich puneetlath


text:
Fix: IOU-Unable to create distance request after deleting finish waypoint, error displayed

Details

https://github.com/Expensify/App/issues/32908
Fixed Issues

$ https://github.com/Expensify/App/issues/32908
PROPOSAL: https://github.com/Expensify/App/issues/32908#issuecomment-1851801714
Tests

Create distance request with 3 waypoints
Goto that created request and remove any waypoint
Save the updated request
Verify it gets saved properly without any error

[X] Verify that no errors appear in the JS console

Offline tests

Create distance request with 3 waypoints
Goto that created request and remove any waypoint
Save the updated request
Verify it gets saved properly without any error

QA Steps

Create distance request with 3 waypoints
Goto that created request and remove any waypoint
Save the updated request
Verify it gets saved properly without any error

[X] Verify that no errors appear in the JS console

PR Author Checklist

[X] I linked the correct issue in the ### Fixed Issues section above
[X] I wrote clear testing steps that cover the changes made in this PR

[X] I added steps for local testing in the Tests section
[X] I added steps for the expected offline behavior in the Offline steps section
[X] I added steps for Staging and/or Production testing in the QA steps section
[X] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[X] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[X] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[X] I included screenshots or videos for tests on all platforms
[X] I ran the tests on all platforms & verified they passed on:

[X] Android / native
[X] Android / Chrome
[X] iOS / native
[X] iOS / Safari
[X] MacOS / Chrome / Safari
[X] MacOS / Desktop

[X] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[X] I followed proper code patterns (see Reviewing the code)

[X] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[X] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[X] I verified that comments were added to code that is not self explanatory
[X] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[X] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[X] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[X] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[X] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[X] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[X] I verified the JSDocs style guidelines (in STYLE.md) were followed

[X] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[X] I followed the guidelines as stated in the Review Guidelines
[X] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[X] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[X] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[X] I verified that if a function's arguments changed that all usages have also been updated correctly
[X] If a new component is created I verified that:

[X] A similar component doesn't exist in the codebase
[X] All props are defined accurately and each prop has a /** comment above it */
[X] The file is named correctly
[X] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[X] The only data being stored in the state is data necessary for rendering and nothing else
[X] If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
[X] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[X] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[X] All JSX used for rendering exists in the render method
[X] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[X] If any new file was added I verified that:

[X] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[X] If a new CSS style is added I verified that:

[X] A similar style doesn't already exist
[X] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))

[X] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[X] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[X] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[X] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[X] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[X] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[X] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
https://github.com/Expensify/App/assets/14358475/fb45f8e4-2caa-4641-9ea8-63e19d05b40d

Mobile Web - Chrome
https://github.com/Expensify/App/assets/14358475/292be642-35ab-4f6a-a0f8-37b2336b1f61

Mobile Web - Safari
https://github.com/Expensify/App/assets/14358475/2ef38f8f-938b-4239-b857-ed14c8f4c809

Desktop
https://github.com/Expensify/App/assets/14358475/1c01f273-f663-4318-9559-c6a7ee3baccb

iOS
https://github.com/Expensify/App/assets/14358475/3bb154b4-003d-49db-8235-532621015d43

Android
https://github.com/Expensify/App/assets/14358475/6421a6cd-cb94-482f-8b35-3f10d4a3527f

Nice job!

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

🚀 Cherry-picked to staging by https://github.com/luacmartins in version: 1.4.11-20 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
failure ❌

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Cherry-picked to staging by https://github.com/luacmartins in version: 1.4.11-20 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Deployed to production by https://github.com/Julesssss in version: 1.4.11-25 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅

authors: Gonals OSBotify Pujan92


text:
[CP Staging] Revert "fix: treat certain US numbers as invalid"

Details
Reverts Expensify/App#31726 and https://github.com/Expensify/App/pull/31010
We have been trying to trace down the rootcause of App crashing for some Expensifiers when opening a search component and these two PRs seem to be the root cause.
When the search page is opened, the option rows are rendered and we run the checks for phone numbers in case the user has a primary login as phone number. Adding this regex and check in to the loop seems to have increased the load so much that for the biggest of account who know many users with phone number as their primary login this
Fixed Issues
$ https://github.com/Expensify/App/issues/33326
PROPOSAL: N/A
Tests

Open the search page
Verify it does not crash

Offline tests
N/A
QA Steps
Will be tested by the largest accounts in Expensify to make sure the Search page is not crashing the App
PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos
Straight reverts, tested in the Slack thread linked above

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

:test_tube::test_tube: Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! :test_tube::test_tube:

Android :robot:
iOS :apple:

https://ad-hoc-expensify-cash.s3.amazonaws.com/android/33496/index.html
https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/33496/index.html

Desktop :computer:
Web :spider_web:

https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/33496/NewExpensify.dmg
https://33496.pr-testing.expensify.com

@aimane-chnaif this one https://github.com/Expensify/App/issues/33326 the app is crashing for accounts which have lots of chats with users who have phone numbers as their primary login

Straight reverts

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

🚀 Cherry-picked to staging by https://github.com/mountiny in version: 1.4.16-4 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Deployed to production by https://github.com/mountiny in version: 1.4.16-5 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

authors: OSBotify mountiny thienlnam