feat: Optimize rendering performance in componentDidMount using reque…

…stAnimationFrame (#3479) Co-authored-by: Ryan Ruan <[email protected]>
Skyscanner · May 28, 2024 · 380266d · 380266d
1 parent 5029705
commit 380266d
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 28 deletions.
diff --git a/packages/bpk-component-horizontal-nav/src/BpkHorizontalNav.js b/packages/bpk-component-horizontal-nav/src/BpkHorizontalNav.js
@@ -65,7 +65,9 @@ class BpkHorizontalNav extends Component<Props> {
   }
 
   componentDidMount() {
-    this.scrollSelectedIntoView(false);
+    requestAnimationFrame(() => {
+      this.scrollSelectedIntoView(false);
+    })
   }
 
   componentDidUpdate() {

diff --git a/packages/bpk-component-mobile-scroll-container/src/BpkMobileScrollContainer.js b/packages/bpk-component-mobile-scroll-container/src/BpkMobileScrollContainer.js
@@ -123,8 +123,10 @@ class BpkMobileScrollContainer extends Component<Props, State> {
   }
 
   componentDidMount() {
-    this.setScrollBarAwareHeight();
-    this.setScrollIndicatorClassName();
+    requestAnimationFrame(() => {
+      this.setScrollBarAwareHeight();
+      this.setScrollIndicatorClassName();
+    });
     window.addEventListener('resize', this.onWindowResize);
   }
 

diff --git a/packages/bpk-scrim-utils/src/withScrim-test.tsx b/packages/bpk-scrim-utils/src/withScrim-test.tsx
@@ -118,6 +118,11 @@ describe('BpkScrim', () => {
     beforeEach(() => {
       TestComponent = () => <div>TestComponent</div>;
       Component = withScrim(TestComponent);
+      jest.useFakeTimers();
+    });
+
+    afterEach(() => {
+      jest.useRealTimers();
     });
 
     it('should mount correctly when is iPhone', () => {
@@ -134,6 +139,7 @@ describe('BpkScrim', () => {
           isIphone
         />,
       );
+      jest.runAllTimers();
       expect(storeScroll).toHaveBeenCalled();
       expect(lockScroll).toHaveBeenCalled();
       expect(fixBody).toHaveBeenCalled();
@@ -155,6 +161,7 @@ describe('BpkScrim', () => {
           isIphone={false}
         />,
       );
+      jest.runAllTimers();
       expect(lockScroll).toHaveBeenCalled();
       expect(focusStore.storeFocus).toHaveBeenCalled();
       expect(storeScroll).not.toHaveBeenCalled();

diff --git a/packages/bpk-scrim-utils/src/withScrim.tsx b/packages/bpk-scrim-utils/src/withScrim.tsx
@@ -87,31 +87,33 @@ const withScrim = <P extends object>(
       const { getApplicationElement, isIpad, isIphone } = this.props;
       const applicationElement = getApplicationElement();
 
-      /**
-       * iPhones & iPads need to have a fixed body
-       * and scrolling stored to prevent some iOS specific issues occuring
-       *
-       * Issue description:
-       * iOS safari does not prevent scrolling on the underlying content.
-       * Without the below fixes this results in users being able to scroll below any modal or dialog that uses withScrim.
-       *
-       * The fixes can be summaried here: https://markus.oberlehner.net/blog/simple-solution-to-prevent-body-scrolling-on-ios/
-       *
-       * The most dangerous of the fixes below is the fixBody, this function applies changes to the <body> style.
-       * This has the potential to override any custom styles already applied. The assumption here is that no one internally is making these changes to body.
-       *
-       * There is a corresponding set of functions in the componentWillUnmount block that deals with undoing these changes.
-       */
-      if (isIphone || isIpad) {
-        storeScroll();
-        fixBody();
-      }
-      /**
-       * lockScroll and the associated unlockScroll is how we control the scroll behaviour of the application when the scrim is active.
-       * The desired behaviour is to prevent the user from scrolling content behind the scrim. The above iOS fixes are in place because lockScroll alone does not solve due to iOS specific issues.
-       */
-
-      lockScroll();
+      requestAnimationFrame(() => {
+        /**
+         * iPhones & iPads need to have a fixed body
+         * and scrolling stored to prevent some iOS specific issues occuring
+         *
+         * Issue description:
+         * iOS safari does not prevent scrolling on the underlying content.
+         * Without the below fixes this results in users being able to scroll below any modal or dialog that uses withScrim.
+         *
+         * The fixes can be summaried here: https://markus.oberlehner.net/blog/simple-solution-to-prevent-body-scrolling-on-ios/
+         *
+         * The most dangerous of the fixes below is the fixBody, this function applies changes to the <body> style.
+         * This has the potential to override any custom styles already applied. The assumption here is that no one internally is making these changes to body.
+         *
+         * There is a corresponding set of functions in the componentWillUnmount block that deals with undoing these changes.
+         */
+        if (isIphone || isIpad) {
+          storeScroll();
+          fixBody();
+        }
+        /**
+         * lockScroll and the associated unlockScroll is how we control the scroll behaviour of the application when the scrim is active.
+         * The desired behaviour is to prevent the user from scrolling content behind the scrim. The above iOS fixes are in place because lockScroll alone does not solve due to iOS specific issues.
+         */
+
+        lockScroll();
+      });
 
       if (applicationElement) {
         applicationElement.setAttribute('aria-hidden', 'true');