-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix - i accept the expensify terms of service, checkbox is not checked when connected online with plaid #18633
Conversation
…f-Service,-Checkbox-is-not-checked-when-connected-online-with-Plaid-Expensify#17877
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
@youssef-lr Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
I have read the CLA Document and I hereby sign the CLA |
…f-Service,-Checkbox-is-not-checked-when-connected-online-with-Plaid-Expensify#17877
…f-Service,-Checkbox-is-not-checked-when-connected-online-with-Plaid-Expensify#17877
@HezekielT can you fix the GH link, it's not supposed to be GHLink highlighted as mentioned in the PR template it should be the actual link of the PR.
That is why we didn't get assigned for review |
…f-Service,-Checkbox-is-not-checked-when-connected-online-with-Plaid-Expensify#17877
Reviewer Checklist
Screenshots/VideosWebScreen.Recording.2023-05-11.at.22.36.54.movMobile Web - ChromeScreen_Recording_20230511-225244_Chrome.mp4Mobile Web - SafariScreen.Recording.2023-05-11.at.22.42.41.movDesktopScreen.Recording.2023-05-11.at.22.46.37.moviOSScreen.Recording.2023-05-11.at.22.39.28.movAndroidScreen_Recording_20230511-224914_New.Expensify.mp4 |
I'm getting this error on iOS Screen.Recording.2023-05-11.at.15.16.57.movBut weirdly, it isn't present on android |
@eVoloshchak This error also happens when we connect bank accounts manually.
I did some research and I found another issue that reported this error a while ago( #12508 ) and the decision was to close it based on those two comments( #12508 (comment) and #12508(comment) ). So the issue is not because of this PR and I don't think we can do anything about it. Please take a look at that issue and let me know what needs to be done. |
So basically the issue only occurs in ios and it doesn’t happen in staging or production(only in dev) and there is a fix but it hasn’t reached ios yet according to this comment. |
@HezekielT, thanks for attaching those links! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
…f-Service,-Checkbox-is-not-checked-when-connected-online-with-Plaid-Expensify#17877
…f-Service,-Checkbox-is-not-checked-when-connected-online-with-Plaid-Expensify#17877
@madmax330 Friendly bump ^ |
🚀 Deployed to staging by https://github.com/madmax330 in version: 1.3.14-2 🚀
|
🚀 Deployed to staging by https://github.com/madmax330 in version: 1.3.14-2 🚀
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.3.14-14 🚀
|
Details
Fixed Issues
$ #17877
PROPOSAL: #17877 (comment)
Tests
Verify if a checkbox is checked when the user returns to bank account step from company information step.
I accept the Expensify terms of service
should appear before proceeding to Company Information step.Routing number
andAccount number
fields and go back to the previous step.Connect manually
option.Routing number
input field.Expensify Terms of Service
link.Offline tests
Needs an internet connection to connect bank account online. Therefore, it is not applicable.
QA Steps
Same as "Tests" Section above.
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Web
Screencast-iAcceptTermsWeb.mov
Mobile Web - Chrome
Screencast-iAcceptTerms-mWebChrome.webm
Mobile Web - Safari
ios-safari-i-accept-terms.mov
Desktop
Screencast-iAcceptTermsDesktop.mov
iOS
ios-native-i-accept-terms.mov
Android
Screencast-iAcceptTermsAndroidNative.mov