Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add cross platform VisualViewport lib for visual viewport resize #10208

Merged
merged 6 commits into from
Aug 3, 2022
Merged

Add cross platform VisualViewport lib for visual viewport resize #10208

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
776428d
Add VisualViewport lib with addResizeListener
neil-marcellini Aug 2, 2022
f56ea8d
Use VisualViewport lib in cross platform component
neil-marcellini Aug 2, 2022
f4e7e2a
Add Viewport to addResizeListener name for clarity
neil-marcellini Aug 2, 2022
c6e9645
Remove unused onViewportResize on native platforms
neil-marcellini Aug 2, 2022
9201c0f
Bind this to updateViewportOffsetTop correctly
neil-marcellini Aug 3, 2022
04f48f7
Merge branch 'main' into neil-visual-viewport-lib
neil-marcellini Aug 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/libs/VisualViewport/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Add a visual viewport resize listener if available. Return a function to remove the listener.
*
* @param {Function} onViewportResize
* @returns {Function}
*/
function addViewportResizeListener(onViewportResize) {
if (!window.visualViewport) {
return () => {};
}

window.visualViewport.addEventListener('resize', onViewportResize);
return () => window.visualViewport.removeEventListener('resize', onViewportResize);
}

export default addViewportResizeListener;
11 changes: 11 additions & 0 deletions src/libs/VisualViewport/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@

/**
* Visual Viewport is not available on native, so return an empty function.
*
* @returns {Function}
*/
function addViewportResizeListener() {
return () => {};
}

export default addViewportResizeListener;
10 changes: 4 additions & 6 deletions src/pages/home/ReportScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import ReportActionsSkeletonView from '../../components/ReportActionsSkeletonVie
import reportActionPropTypes from './report/reportActionPropTypes';
import ArchivedReportFooter from '../../components/ArchivedReportFooter';
import toggleReportActionComposeView from '../../libs/toggleReportActionComposeView';
import addViewportResizeListener from '../../libs/VisualViewport';

const propTypes = {
/** Navigation route context info provided by react navigation */
Expand Down Expand Up @@ -114,6 +115,7 @@ class ReportScreen extends React.Component {

this.onSubmitComment = this.onSubmitComment.bind(this);
this.viewportOffsetTop = this.updateViewportOffsetTop.bind(this);
this.removeViewportResizeListener = () => {};

this.state = {
skeletonViewContainerHeight: 0,
Expand All @@ -123,9 +125,7 @@ class ReportScreen extends React.Component {

componentDidMount() {
this.storeCurrentlyViewedReport();
if (window.visualViewport) {
window.visualViewport.addEventListener('resize', this.viewportOffsetTop);
}
this.removeViewportResizeListener = addViewportResizeListener(this.viewportOffsetTop);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm actually this is a pretty bad name for this function. Can we give it a better one while we are here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I am confused though this looks like a variable and not a function

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see it now we did this:

        this.viewportOffsetTop = this.updateViewportOffsetTop.bind(this);

yeah that should be

        this.updateViewportOffsetTop = this.updateViewportOffsetTop.bind(this);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep good point, that's confusing. I'll change it.

}

componentDidUpdate(prevProps) {
Expand All @@ -137,9 +137,7 @@ class ReportScreen extends React.Component {

componentWillUnmount() {
clearTimeout(this.loadingTimerId);
if (window.visualViewport) {
window.visualViewport.removeEventListener('resize', this.viewportOffsetTop);
}
this.removeViewportResizeListener();
}

/**
Expand Down