Added scrollable to payments page and connect bank account page#12935
Conversation
|
@marcochavezf @eVoloshchak One of you needs to 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] |
Reviewer Checklist
Screenshots/VideosWebScreen.Recording.2022-11-23.at.15.06.35.movMobile Web - Chrome22-11-23-19-05-15.mp4Mobile Web - SafariScreen.Recording.2022-11-23.at.19.00.00.movDesktopScreen.Recording.2022-11-23.at.15.12.37.moviOSScreen.Recording.2022-11-23.at.15.14.55.movAndroid22-11-23-19-12-00.mp4 |
eVoloshchak
left a comment
There was a problem hiding this comment.
Looks good and tests well
cc: @marcochavezf
|
@jayeshmangwani, could you resolve the conflicts please? |
@eVoloshchak conflicts resolved |
eVoloshchak
left a comment
There was a problem hiding this comment.
LGTM
cc: @marcochavezf
marcochavezf
left a comment
There was a problem hiding this comment.
LGTM, thank you guys!
|
✋ 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 @marcochavezf in version: 1.2.32-0 🚀
|
|
@marcochavezf @AndrewGable While executing this PR we noticed on Desktop that Payments page and Connect bank account page are not scrollable. Is it expected by design or should we log an issue? Screen.Recording.2022-11-26.at.16.15.29.mov |
|
@mvtglobally I think it makes sense that the page is not scrollable if it the content does not overflow |
|
Agree with @mountiny |
|
🚀 Deployed to production by @luacmartins in version: 1.2.32-2 🚀
|
|
🚀 Deployed to production by @luacmartins in version: 1.2.32-2 🚀
|
Details
Fixed Issues
$ #11695
PROPOSAL: #11695 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
I linked the correct issue in the
### Fixed Issuessection aboveI wrote clear testing steps that cover the changes made in this PR
TestssectionOffline stepssectionQA stepssectionI included screenshots or videos for tests on all platforms
I ran the tests on all platforms & verified they passed on:
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)
I followed proper code patterns (see Reviewing the code)
toggleReportand notonIconClick)src/languages/*filesWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedIf a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I followed the guidelines as stated in the Review Guidelines
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 usingAvatarare working as expected)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
I verified that if a function's arguments changed that all usages have also been updated correctly
If a new component is created I verified that:
/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)If a new CSS style is added I verified that:
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
Avataris modified, I verified thatAvataris 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 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
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarhave been tested & I retested again)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)Screenshots
Web
https://drive.google.com/file/d/15OLV_9Rq1QdkFra9itHgQdHCdbtRxnhL/view?usp=sharing
Mobile Web - Chrome
Android-chrome-11695.mp4
Mobile Web - Safari
https://drive.google.com/file/d/18m2p_cQbbN8XQ50qlLhlRlbq3XTi2ZlD/view?usp=sharing
Desktop
https://drive.google.com/file/d/1U17A66ALC8x1t196i-o4LDupSrqFyAGM/view?usp=sharing
iOS
ios-native-11695.mov
Android
Android-native-11695.mp4