Fix: Modal freezing issue on Payments page #8173
Conversation
There was a problem hiding this comment.
@parasharrajat Changes look good to me, but I am just wondering since this is a workaround for the case of more than one modal in RN, should we document this approach a bit more thoroughly at some readme? Also maybe some comments for the Interaction Manager would not hurt so it is a bit clearer what is going on just by reading the codebase.
Essentially, I believe we would run into this bug in future as we will add features and functionality, therefore leaving some instructions for other contributors would be handy. What do you think?
|
Sure. Apart from this Please let me know where do you want me to document it. I will add it. |
|
@parasharrajat Alright, I thought there was a markdown file with the React Navigation steps (not sure why 😄 ) but I cant find any so I think it would be fine if you could just write a bit more detailed comment in this file to explain it. Surely if a similar problem comes again, you or I will be able to point to this file as an example of what to do! |
| @@ -364,7 +377,7 @@ class BasePaymentsPage extends React.Component { | |||
| isDangerousAction | |||
| /> | |||
| <ConfirmPopover | |||
| contentStyles={[!this.props.isSmallScreenWidth ? styles.sidebarPopover : '']} | |||
| contentStyles={!this.props.isSmallScreenWidth ? [styles.sidebarPopover] : undefined} | |||
There was a problem hiding this comment.
Why change to undefined instead of [""] which was there previously? Just curious.
Otherwise just missing the comments and we should be good to go.
There was a problem hiding this comment.
Setting it to undefined is like resetting it. So it will automatically take dfaultProp value for this prop.
[""] is the wrong value for this prop. It expects it to be Array<Object>.
There was a problem hiding this comment.
Gotcha! thanks for the explanation.
|
I have added the comments. Please let me know if you see some improvements. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
@parasharrajat @mountiny @fukawi2 We are still experience the open issue in IOS app #7869 HBWZ7385.1.MP4 |
|
@parasharrajat I could confirm on my account on the staging app that the first case of just opening the modal and closing it by clicking outside (iOS) worked well. However, when I clicked make default, the modal closed, but the app froze. Unfortunately cannot provide any logs from that. It is interesting though that it works for one case and not for the other. |
|
Ok I will take a stab at it. |
|
@parasharrajat @mountiny any updates on this? I don't think we need to hold the deploy based that this is partially fixed now and the original problem is on production. Does that sound right to you two? |
|
@timszot definitely sounds right to me! |
|
Yup Sounds good. |
Details
Fixed Issues
$ #7768
Tests
PR Review Checklist
Contributor (PR Author) Checklist
main### Fixed Issuessection abovesrc/languages/*files (if applicable)Styling.md) for all style edits I madeSTYLE.md)Avatar, I verified the components usingAvatarare working as expected)mainbranch)PR Reviewer Checklist
### Fixed Issuessection aboveTestssectionQA stepssectionsrc/languages/*files (if applicable)STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)displayNameproperty if it’s a Functional Component(i.e.
StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)QA Steps
Make default payment Method.Deleteoption.Run these steps on staging on all platforms. Note: pressing the Back hardware button on the Android web is not the same as pressing the back button Android native app. To close the modal on Android Web click outside the area.
Screenshots
Web | Desktop
Mobile Web
iOS
screen-2022-03-16_22.05.55.mp4
Android