Add cross platform VisualViewport lib for visual viewport resize#10208
Conversation
marcaaron
left a comment
There was a problem hiding this comment.
Looks great thanks for making this change 🙇
| * @param {Function} onViewportResize | ||
| * @returns {Function} | ||
| */ | ||
| // eslint-disable-next-line no-unused-vars |
There was a problem hiding this comment.
Can just remove the param if we're not using it
There was a problem hiding this comment.
I wasn't sure what was Kosher here, but sounds good, I'll do that.
| if (window.visualViewport) { | ||
| window.visualViewport.addEventListener('resize', this.viewportOffsetTop); | ||
| } | ||
| this.removeViewportResizeListener = addViewportResizeListener(this.viewportOffsetTop); |
There was a problem hiding this comment.
Hmm actually this is a pretty bad name for this function. Can we give it a better one while we are here?
There was a problem hiding this comment.
Maybe I am confused though this looks like a variable and not a function
There was a problem hiding this comment.
Oh I see it now we did this:
this.viewportOffsetTop = this.updateViewportOffsetTop.bind(this);yeah that should be
this.updateViewportOffsetTop = this.updateViewportOffsetTop.bind(this);There was a problem hiding this comment.
Yep good point, that's confusing. I'll change it.
|
✋ 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 @marcaaron in version: 1.1.88-0 🚀
|
cc @marcaaron
Details
Instead of checking for
window.visualViewportin a cross platform component, create a small lib to add the event listener on web and do nothing on native platforms.Fixed Issues
$ #10204
Tests
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNamepropertythisproperly 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)PR Reviewer Checklist
The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarhave been tested & I retested again)/** comment above it */displayNamepropertythisproperly 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)QA Steps
Same as tests.
Screenshots
Web
Mobile Web
iOS-Safari.mp4
Desktop
iOS
iOS.mp4
Android